Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/typescript-fix-in-skip-reasons.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@astryxdesign/cli': patch
---

[feat] The doc-type and prop checks in `integration check` and `doctor integration validate|components` read types with `@typescript/typescript6`, the TypeScript 6 API package TypeScript publishes for tools, when the project installs it and its own `typescript` is missing or has no JavaScript compiler API, as TypeScript 7 does. When no usable TypeScript is found, the skip reason now ends with the remedy: add `@typescript/typescript6` on TypeScript 7, or install TypeScript 5.0 through 6 (`npm install -D typescript@6`).
@josephfarina
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ export const doc = {
'replaces is checked too: its findings are issues, errors for a package that ' +
'declares the CLI range that turns replacement on and warnings otherwise. ' +
"Each component doc's props are compared with the component's TypeScript props, " +
"read with the project's own `typescript`: a documented prop the code lacks, a " +
"read with the project's own `typescript` (or, where `typescript` is missing or has no " +
'JavaScript compiler API, as TypeScript 7 does, the `@typescript/typescript6` it ' +
'installs): a documented prop the code lacks, a ' +
'prop the package declares that the doc omits, a wrong `required`, or a type ' +
'that does not match. A type is judged only when it reads exactly: the same ' +
'text as the source, once whitespace, quotes and union order are set aside (exactly, if it contains `readonly`), ' +
Expand Down Expand Up @@ -50,14 +52,18 @@ export const doc = {
'drift',
],
params: [
{name: 'pkg', type: 'string', description: 'Installed package; omit for the local package.'},
{
name: 'pkg',
type: 'string',
description: 'Installed package; omit for the local package.',
},
{name: 'options.cwd', type: 'string', description: 'Resolution directory.'},
],
returns: [
{
type: 'integration.component-conflicts',
description:
'`validated` (false when no integration manifest was found, so nothing was inspected), integration identity, structural and replaces issues, and Core component-name conflicts with package-qualified commands. An active replacement that keeps the Core name it replaces is not a conflict. When the manifest loads and declares a components root, `propDrift` holds the props comparison: `status` (`checked` | `skipped`), `reason` when skipped, `typescript`, `components` compared, `typesCompared` (documented prop types judged), `typesSkipped` (why none was, with a TypeScript older than 4.8), `findings` ({component, prop, kind: `not_in_code` | `not_in_doc` | `required_mismatch` | `type_mismatch`, severity: `warning`, message, doc, source, docType?, codeType?, docRequired?, codeRequired?}), `unchecked` ({component, doc, reason}), `typesNotCompared` ({component, doc, reason, props}: every other documented prop type, grouped by why it was not judged: not a form the check judges exactly (a doc type that does not parse, or a prop a mapped type builds from keys the check cannot trace, included), a field the prop\'s variants give different types, a standard prop such as `className`, or a prop declared outside the package), and `propsNotChecked` when a mapped type builds props from keys the check cannot trace to the package ({component, doc, reason, props}: no warning is drawn for those props).',
"`validated` (false when no integration manifest was found, so nothing was inspected), integration identity, structural and replaces issues, and Core component-name conflicts with package-qualified commands. An active replacement that keeps the Core name it replaces is not a conflict. When the manifest loads and declares a components root, `propDrift` holds the props comparison: `status` (`checked` | `skipped`), `reason` when skipped, `typescript`, `components` compared, `typesCompared` (documented prop types judged), `typesSkipped` (why none was, with a TypeScript older than 4.8), `findings` ({component, prop, kind: `not_in_code` | `not_in_doc` | `required_mismatch` | `type_mismatch`, severity: `warning`, message, doc, source, docType?, codeType?, docRequired?, codeRequired?}), `unchecked` ({component, doc, reason}), `typesNotCompared` ({component, doc, reason, props}: every other documented prop type, grouped by why it was not judged: not a form the check judges exactly (a doc type that does not parse, or a prop a mapped type builds from keys the check cannot trace, included), a field the prop's variants give different types, a standard prop such as `className`, or a prop declared outside the package), and `propsNotChecked` when a mapped type builds props from keys the check cannot trace to the package ({component, doc, reason, props}: no warning is drawn for those props).",
},
],
examples: [
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/api/integration/validateIntegration.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ export const doc = {
'can stay exit-0 in a non-integration directory. It also checks every doc the ' +
'package ships under its roots (component, template, docs topic, theme, and translation, served or not) against ' +
"the published authoring type for its kind, as `tsc` would, with the project's " +
'own `typescript`: an unknown field, a missing required field, or a value outside ' +
'own `typescript` (or, where `typescript` is missing or has no JavaScript compiler ' +
'API, as TypeScript 7 does, the `@typescript/typescript6` it installs): an unknown ' +
'field, a missing required field, or a value outside ' +
"a field's type is a warning in `docTypes` naming the field and a fix. A test " +
'fixture under `__tests__` in the components or docs root is counted, not checked. ' +
'Without TypeScript, `docTypes` is `skipped` with a reason.',
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/assets/docs/tree/check-the-props.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ docType: boolean
style: 'unordered',
items: [
"A doc's own `props` describe the export its `name` names. In a family doc, each entry describes its own export. A hook entry documents parameters and returns, so it is not compared.",
"It reads props with the `typescript` your package installs, and reads your package's `tsconfig.json` the way `tsc -p` does. Without TypeScript it prints `[skip] Props not compared` and the reason.",
"It reads props with the `typescript` your package installs, and reads your package's `tsconfig.json` the way `tsc -p` does. Where `typescript` is missing or has no JavaScript compiler API, as TypeScript 7 does, it uses `@typescript/typescript6`, the TypeScript 6 API package, if your package installs it. Without either it prints `[skip] Props not compared` and the reason.",
'Every finding is a warning, so the exit code stays 0. Read them before you publish. With `--json`, they are in `data.propDrift.findings`.',
],
},
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/assets/docs/tree/check-your-docs.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fix: Remove \`summary\`: SingleComponentDoc has no field \`summary\`.`,
items: [
"The folder decides the type: `ComponentDoc` in your components root, `IntegrationTemplateDoc` in your templates root, `ThemeDoc` in your themes root, and in your docs root the type the doc's `type` names. Keep the `@type` comment above each doc's export too, so your editor shows the same problems as you type.",
'Every finding is a warning, so the exit code stays 0. With `--json`, they are in `data.docTypes.findings`.',
'It uses the `typescript` your package installs. Without it, it prints `[skip] Docs not type-checked` and the reason.',
'It uses the `typescript` your package installs. Where `typescript` is missing or has no JavaScript compiler API, as TypeScript 7 does, it uses `@typescript/typescript6`, the TypeScript 6 API package, if your package installs it. Without either, it prints `[skip] Docs not type-checked` and the reason.',
'Every field of every doc type is in `astryx docs authoring`.',
],
},
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/assets/docs/tree/checks.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const docs = {
'`npx astryx doctor integration validate`',
'The manifest loads, each root holds contributions the CLI can read, and each doc matches its authoring type',
'A declared root is missing (`missing_root`), a contribution does not load (`invalid_doc`, `invalid_component`, `invalid_theme`), or two templates in the package replace one Core id (`ambiguous_template_replacement`)',
"The manifest has a key this CLI does not know (`unknown_manifest_key`), or a doc field does not match the doc's authoring type: a field the type does not have, a missing required field, or a value outside the field's type. It checks docs with your installed `typescript`; without it, it prints `[skip] Docs not type-checked`. With no `astryx.integration.mjs` it prints a hint and exits 0",
"The manifest has a key this CLI does not know (`unknown_manifest_key`), or a doc field does not match the doc's authoring type: a field the type does not have, a missing required field, or a value outside the field's type. It checks docs with your installed `typescript`, or, where that is missing or has no JavaScript compiler API (TypeScript 7), with an installed `@typescript/typescript6`; without either, it prints `[skip] Docs not type-checked`. With no `astryx.integration.mjs` it prints a hint and exits 0",
],
[
'`npx astryx doctor integration components`',
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/assets/docs/tree/quick-start.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const docs = {
'Start with `"exports": {}`: each component and template you add then writes the public import that `integration verify` resolves.',
'Run the CLI as `npx astryx`, which runs the `@astryxdesign/cli` you installed as a devDependency.',
'Component commands read Core, so they need `@astryxdesign/core` installed.',
"The checks compare your docs with your code through TypeScript's JavaScript compiler API, so they need `typescript` 5.0 through 6 installed. TypeScript 7 does not ship that API, and with it the checks skip those comparisons.",
"The checks compare your docs with your code through TypeScript's JavaScript compiler API, which `typescript` 5.0 through 6 has. TypeScript 7 does not ship that API: on TypeScript 7, also install `@typescript/typescript6`, its TypeScript 6 API package, and the checks use it.",
],
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ export const doc = {
'Core component is an error. In a package without that range they are warnings, ' +
'with a warning for each such component, naming the range to declare. ' +
"Then it compares each component doc's props with the component's TypeScript " +
"props, using the project's own `typescript`, and warns about a documented prop " +
"props, using the project's own `typescript` (or, where `typescript` is missing or " +
'has no JavaScript compiler API, as TypeScript 7 does, the `@typescript/typescript6` ' +
'it installs), and warns about a documented prop ' +
'the code lacks, a prop the package declares that the doc omits, a wrong ' +
'`required`, and a type that does not match where both sides read exactly ' +
'(it lists every other documented type as not compared, with the reason). ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export const doc = {
'installed package resolved by name. It schema-checks the manifest, verifies ' +
'every declared contribution root, and reports every finding. Safe as a CI gate. ' +
'It then checks every doc the package ships under its roots against the ' +
"published authoring type for its kind, with the project's own `typescript`, and " +
"published authoring type for its kind, with the project's own `typescript` (or, " +
'where `typescript` is missing or has no JavaScript compiler API, as TypeScript 7 ' +
'does, the `@typescript/typescript6` it installs), and ' +
'warns about each field that does not match, naming the field and a fix. A doc ' +
'it cannot fully check is listed with the reason, and test fixtures under ' +
'`__tests__` in the components or docs root are counted, not checked. Without ' +
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/foundation/discovery/doc-types.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import * as path from 'node:path';
import {fileURLToPath} from 'node:url';
import {
resolveProjectTypeScript,
TYPESCRIPT_FIX,
unusableTypeScriptReason,
} from './project-typescript.mjs';

Expand Down Expand Up @@ -91,8 +92,7 @@ import {
*/

/** The line both surfaces print when the compiler is missing. */
export const DOC_TYPES_TYPESCRIPT_MISSING_REASON =
'TypeScript is not installed in this project, so docs were not checked against the authoring types. Install typescript and run this check again.';
export const DOC_TYPES_TYPESCRIPT_MISSING_REASON = `TypeScript is not installed in this project, so docs were not checked against the authoring types. ${TYPESCRIPT_FIX}`;

const AUTHORING = '@astryxdesign/cli/authoring';
const AUTHORING_TYPES = path.resolve(
Expand Down Expand Up @@ -933,7 +933,7 @@ export async function checkDocTypes(files, options) {
if (major < needMajor || (major === needMajor && minor < needMinor)) {
return {
status: 'skipped',
reason: `TypeScript ${version} cannot check ${hasJs ? 'a JavaScript doc' : 'a doc'} against its type (that needs TypeScript ${needMajor}.${needMinor} or later), so docs were not checked. Upgrade typescript and run this check again.`,
reason: `TypeScript ${version} cannot check ${hasJs ? 'a JavaScript doc' : 'a doc'} against its type (that needs TypeScript ${needMajor}.${needMinor} or later), so docs were not checked. ${TYPESCRIPT_FIX}`,
docs: 0,
findings: [],
unchecked: [],
Expand Down
48 changes: 48 additions & 0 deletions packages/cli/foundation/discovery/doc-types.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,37 @@ function withTypeScript() {
);
}

/**
* TypeScript 7 (no JavaScript compiler API) with the TypeScript 6 API package
* beside it, which re-exports a real TypeScript.
*/
function withTypeScript7AndItsTypeScript6Api() {
const ts7 = path.join(pkg, 'node_modules', 'typescript');
fs.mkdirSync(ts7, {recursive: true});
fs.writeFileSync(
path.join(ts7, 'package.json'),
JSON.stringify({
name: 'typescript',
version: '7.0.2',
exports: {'./package.json': './package.json'},
}),
);
const ts6 = path.join(pkg, 'node_modules', '@typescript', 'typescript6');
fs.mkdirSync(ts6, {recursive: true});
fs.writeFileSync(
path.join(ts6, 'package.json'),
JSON.stringify({
name: '@typescript/typescript6',
version: '6.0.2',
main: 'index.js',
}),
);
fs.writeFileSync(
path.join(ts6, 'index.js'),
`module.exports = require(${JSON.stringify(TYPESCRIPT_DIR)});\n`,
);
}

/**
* @param {string} rel
* @param {string} text
Expand Down Expand Up @@ -117,6 +148,10 @@ describe('checkDocTypes', () => {

const report = await check([['components/Badge.doc.mjs', 'components']]);

// The reason names the version and the fix, so a --strict failure
// says what to install.
expect(report.reason).toContain('TypeScript 7.0.0');
expect(report.reason).toContain('npm install -D @typescript/typescript6');
expect(report).toEqual({
status: 'skipped',
reason: unusableTypeScriptReason('7.0.0', 'docs were not checked'),
Expand All @@ -128,6 +163,19 @@ describe('checkDocTypes', () => {
}
});

it("checks with TypeScript 7's TypeScript 6 API package when the project has one", async () => {
write(
'components/Badge.doc.mjs',
`export default ${JSON.stringify(COMPONENT)};\n`,
);
withTypeScript7AndItsTypeScript6Api();

const report = await check([['components/Badge.doc.mjs', 'components']]);

expect(report.status).toBe('checked');
expect(report.docs).toBe(1);
});

it('says there is nothing to check when the package has no doc files, with or without TypeScript', async () => {
const without = await check([]);
withTypeScript();
Expand Down
36 changes: 32 additions & 4 deletions packages/cli/foundation/discovery/project-typescript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,16 @@ import * as path from 'node:path';
import {createRequire} from 'node:module';

/**
* Resolve the project's own `typescript`, trying each directory in order.
* The package TypeScript publishes beside TypeScript 7 for tools that need a
* JavaScript compiler API: TypeScript 6's. Kept in a constant so the type
* check does not treat the load as a static import of an uninstalled package.
*/
const TYPESCRIPT_6_API = '@typescript/typescript6';

/**
* Resolve the project's own `typescript`, trying each directory in order;
* where it has no JavaScript compiler API (TypeScript 7), the TypeScript 6
* API package installed beside it.
* @param {Array<string | null | undefined>} dirs
* @returns {{ts: any, version: string} | {ts: null, version: string} | null}
* `ts: null` when a TypeScript is installed but has no `createProgram`
Expand Down Expand Up @@ -43,19 +52,38 @@ export function resolveProjectTypeScript(dirs) {
unusable ??= installed();
} catch {
// Not installed from here, or installed without an entry this CLI can
// load; try the next directory.
// load.
unusable ??= installed();
}
// TypeScript 7 has no JavaScript compiler API; the package TypeScript
// ships beside it for tools that need one carries TypeScript 6's.
try {
const ts = require(TYPESCRIPT_6_API);
if (ts && typeof ts.createProgram === 'function') {
return {ts, version: String(ts.version)};
}
} catch {
// Not installed from here; try the next directory.
}
Comment on lines +58 to +67

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback is automatic CLI behavior, so AST-017 FR14 needs it documented and discoverable, but the help and guides still say the checks read the project's own typescript (doctor-integration-components.doc.mjs, doctor-integration-validate.doc.mjs, integrationComponentConflicts.doc.mjs, validateIntegration.doc.mjs, check-the-props.doc.mjs, check-your-docs.doc.mjs, checks.doc.mjs), and quick-start.doc.mjs:52 says TypeScript 7 makes the checks skip. Please describe it there.

This require also runs when no typescript resolves from the folder (the catch above falls through to it), so a package with only @typescript/typescript6 is checked with it too; the docs should say so.

}
return unusable ? {ts: null, version: unusable} : null;
}

/**
* What to do when the project's TypeScript is missing or cannot be used:
* beside TypeScript 7, the TypeScript 6 API package it ships for tools;
* otherwise a version every check here reads, 5.0 (the oldest that checks a
* JavaScript doc) through 6.
*/
export const TYPESCRIPT_FIX =
'On TypeScript 7, add its TypeScript 6 API package (`npm install -D @typescript/typescript6`); otherwise install TypeScript 5.0 through 6 (`npm install -D typescript@6`). Then run this check again.';

/**
* Why a check that needs the compiler did not run, when the TypeScript
* installed has no JavaScript compiler API.
* installed has no JavaScript compiler API, and what to do.
* @param {string} version
* @param {string} outcome what was not done, such as `docs were not checked`
*/
export function unusableTypeScriptReason(version, outcome) {
return `TypeScript ${version} is installed in this project, but it has no JavaScript compiler API (\`createProgram\`) to read types with, so ${outcome}.`;
return `TypeScript ${version} is installed in this project, but it has no JavaScript compiler API (\`createProgram\`) to read types with (TypeScript 7 ships none), so ${outcome}. ${TYPESCRIPT_FIX}`;
}
4 changes: 2 additions & 2 deletions packages/cli/foundation/discovery/prop-drift.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {loadComponentDoc} from './component-loader.mjs';
import {readPackageTsconfig} from './package-tsconfig.mjs';
import {
resolveProjectTypeScript,
TYPESCRIPT_FIX,
unusableTypeScriptReason,
} from './project-typescript.mjs';
import {judgeType} from './type-match.mjs';
Expand Down Expand Up @@ -158,8 +159,7 @@ const outside = (name, code) =>
const HOOK_NAME = /^use[A-Z0-9]/;

/** The line both surfaces print when the compiler is missing. */
export const TYPESCRIPT_MISSING_REASON =
'TypeScript is not installed in this project, so documented props were not compared with the code. Install typescript and run this check again.';
export const TYPESCRIPT_MISSING_REASON = `TypeScript is not installed in this project, so documented props were not compared with the code. ${TYPESCRIPT_FIX}`;

/**
* @template T
Expand Down
Loading
Loading