diff --git a/.changeset/cli-component-doc-names-another-export.md b/.changeset/cli-component-doc-names-another-export.md new file mode 100644 index 0000000000000..e7a29de42e752 --- /dev/null +++ b/.changeset/cli-component-doc-names-another-export.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/cli': patch +--- + +[feat] An integration component doc can name a differently named export of the file beside it: when `Slides.tsx` exports `AcmeCarousel` and nothing named `Slides`, `Slides.doc.mjs` can set `name: 'AcmeCarousel'`, and a kebab-case `acme-widget.doc.mjs` can name `AcmeWidget`. `component --list`, `component `, `search`, `swizzle`, the `doctor integration` checks and `integration verify` know the component by that name, and its file name still opens it. A name that Core or another doc's file already has stays theirs on every surface unless `--package` names the integration. Docs and sources are read without running them; a doc named after its file, or beside a file that exports its own name, reads exactly as before. + +@josephfarina diff --git a/packages/cli/api/component/_adapter.mjs b/packages/cli/api/component/_adapter.mjs index 55e5c995a5fad..e777331a492ae 100644 --- a/packages/cli/api/component/_adapter.mjs +++ b/packages/cli/api/component/_adapter.mjs @@ -28,13 +28,14 @@ import { import { CORE_PACKAGE, discoverComponents, + docStem, findComponentReadme, findComponentSource, findExternalComponentDoc, - findIntegrationComponentDoc, - findIntegrationComponentSource, + findIntegrationComponent, resolveImportPath, resolveIntegrationImportPath as resolveIntegrationImport, + unscopedComponentOwners, } from '../../foundation/discovery/component-discovery.mjs'; import {Project} from '../../foundation/config/project.mjs'; import {loadComponentDoc as loadValidatedComponentDoc} from '../../foundation/discovery/component-loader.mjs'; @@ -42,7 +43,7 @@ import {resolveComponentReplacements} from '../../foundation/discovery/component import {searchComponents} from '../../foundation/text/string-utils.mjs'; import {AstryxError} from '../error.mjs'; -export {CORE_PACKAGE}; +export {CORE_PACKAGE, unscopedComponentOwners}; /** * A loaded component doc. The shared validated loader accepts stamped and legacy @@ -71,6 +72,8 @@ export {CORE_PACKAGE}; * @property {string|null} sourcePath * @property {string|undefined} issuesUrl * @property {import('../../foundation/integrations/integrations.mjs').LoadedIntegration|null} integration + * @property {import('../../foundation/discovery/component-discovery.mjs').NameHold} hold + * how the owner holds the name, for the owner rule */ /** @@ -240,17 +243,19 @@ export function resolveOwners(coreDir, dirName, loadedIntegrations) { sourcePath: findComponentSource(coreDir, dirName), issuesUrl: undefined, integration: null, + hold: 'core', }); } for (const integration of loadedIntegrations) { - const docPath = findIntegrationComponentDoc(integration, dirName); - if (!docPath) continue; + const found = findIntegrationComponent(integration, dirName); + if (!found) continue; owners.push({ package: integration.name, - docPath, - sourcePath: findIntegrationComponentSource(integration, dirName), + docPath: found.docPath, + sourcePath: found.sourcePath, issuesUrl: integration.issuesUrl, integration, + hold: found.hold, }); } return owners; @@ -361,6 +366,39 @@ export function resolveLegacyExternalDoc(ext, dirName) { return extDocPath && extDocPath.endsWith('.doc.mjs') ? extDocPath : null; } +/** + * Whether a legacy external package documents `dirName`. + * @param {string} cwd + * @param {string} dirName + * @returns {boolean} + */ +export function legacyExternalDocuments(cwd, dirName) { + return discoverExternalPackages(cwd).some( + ext => resolveLegacyExternalDoc(ext, dirName) != null, + ); +} + +/** + * Whether a legacy docs package keeps `dirName` over these owners, each of which + * holds it only as an export a doc records. + * @param {string} cwd + * @param {string} dirName + * @param {Array<{hold?: string, docPath?: string | null}>} owners + * @returns {boolean} + */ +export function legacyExternalKeeps(cwd, dirName, owners) { + return ( + owners.length > 0 && + owners.every( + owner => + owner.hold === 'doc' && + owner.docPath != null && + docStem(owner.docPath).toLowerCase() !== dirName.toLowerCase(), + ) && + legacyExternalDocuments(cwd, dirName) + ); +} + /** * The core source file for `dirName` (the no-scope `--source` path reads core * directly — it does NOT fall back to externals or fuzzy search). diff --git a/packages/cli/api/component/component.mjs b/packages/cli/api/component/component.mjs index f0c938bc14c6a..52bce5a8e72d6 100644 --- a/packages/cli/api/component/component.mjs +++ b/packages/cli/api/component/component.mjs @@ -22,6 +22,7 @@ import { classifyScope, assertUnambiguousOwners, resolveLegacyExternalDoc, + legacyExternalKeeps, resolveCoreSourcePath, resolveUnscopedDoc, loadComponentDoc, @@ -29,6 +30,7 @@ import { ComponentAmbiguityError, installedComponentPackageVersion, resolveComponentReplacement, + unscopedComponentOwners, } from './_adapter.mjs'; import {componentList} from './list/list.mjs'; import {componentDetail} from './detail/detail.mjs'; @@ -481,12 +483,14 @@ export async function component(name, options = {}) { }); } + // The owner rule runs first, over valid and invalid owners alike. + const ruled = unscopedComponentOwners(owners); // Invalid integration metadata does not create ambiguity against a valid - // owner. Keep raw owners only when no doc owner is valid, so an integration- + // owner. Keep ruled owners only when no doc owner is valid, so an integration- // only component still exposes its source and returns ERR_INVALID_DOC for // detail instead of degrading to an unrelated unknown-component error. const validOwners = []; - for (const owner of owners) { + for (const owner of ruled) { if (owner.package === CORE_PACKAGE) { validOwners.push(owner); continue; @@ -498,7 +502,12 @@ export async function component(name, options = {}) { // Project/Doctor report invalid metadata; it is not an effective owner. } } - const effectiveOwners = validOwners.length > 0 ? validOwners : owners; + const loadable = validOwners.length > 0 ? validOwners : ruled; + // A legacy docs package's exact doc keeps its name over an export name a doc + // records, as Core and file-named owners do. + const effectiveOwners = legacyExternalKeeps(cwd, dirName, loadable) + ? [] + : loadable; // ── Ambiguity: owned by MORE THAN ONE valid package, no --package ── assertUnambiguousOwners(effectiveOwners, dirName); diff --git a/packages/cli/api/component/integration-export-name.test.mjs b/packages/cli/api/component/integration-export-name.test.mjs new file mode 100644 index 0000000000000..0c6b5f303e327 --- /dev/null +++ b/packages/cli/api/component/integration-export-name.test.mjs @@ -0,0 +1,419 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Every surface knows an integration component by the name its doc + * documents. Fixtures live under a repo-local temp dir: Vite refuses to import + * a module from outside the project root. + */ + +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import {component} from './component.mjs'; +import {search} from '../search/search.mjs'; +import {validateIntegration} from '../integration/validate-integration.mjs'; +import {integrationComponentConflicts} from '../integration/authoring-checks.mjs'; +import {swizzleCopy} from '../swizzle/copy/copy.mjs'; +import {gapReport} from '../gap-report/gap-report.mjs'; +import {COMPONENT_REPLACES_CLI} from '../../foundation/integrations/cli-requirement.mjs'; + +const SLOW = 30_000; + +/** @type {string} */ +let tmpDir; +/** @type {string} */ +let pkgDir; + +/** + * @param {string} file path under the package's src/, without extension + * @param {string} name + * @param {string} source + * @param {string} description + * @param {object} [extra] more doc fields + */ +function writeComponent(file, name, source, description, extra = {}) { + const target = path.join(pkgDir, 'src', file); + fs.mkdirSync(path.dirname(target), {recursive: true}); + fs.writeFileSync(`${target}.tsx`, source); + fs.writeFileSync( + `${target}.doc.mjs`, + `export default ${JSON.stringify( + { + type: 'component', + name, + displayName: name, + import: `@acme/widgets/${file}`, + usage: {description}, + props: [], + ...extra, + }, + null, + 2, + )};\n`, + ); +} + +beforeEach(() => { + tmpDir = fs.mkdtempSync( + path.join(process.cwd(), '.astryx-export-name-test-'), + ); + fs.writeFileSync( + path.join(tmpDir, 'package.json'), + JSON.stringify({name: 'consumer', version: '1.0.0'}), + ); + fs.writeFileSync( + path.join(tmpDir, 'astryx.config.mjs'), + "export default {integrations: ['@acme/widgets']};\n", + ); + pkgDir = path.join(tmpDir, 'node_modules', '@acme', 'widgets'); + fs.mkdirSync(pkgDir, {recursive: true}); + fs.writeFileSync( + path.join(pkgDir, 'package.json'), + JSON.stringify({ + name: '@acme/widgets', + version: '1.0.0', + exports: { + './gallery/Slides': './src/gallery/Slides.tsx', + './gallery/Overlays': './src/gallery/Overlays.tsx', + './AcmeBadge': './src/AcmeBadge.tsx', + }, + }), + ); + fs.writeFileSync( + path.join(pkgDir, 'astryx.integration.mjs'), + "export default {components: './src', issuesUrl: 'https://example.com/acme/issues'};\n", + ); + writeComponent( + 'gallery/Slides', + 'AcmeCarousel', + 'export function AcmeCarousel() { return null; }\nexport function useSlides() { return []; }\n', + 'Cycles through a row of slides.', + ); + writeComponent( + 'AcmeBadge', + 'AcmeBadge', + 'export function AcmeBadge() { return null; }\n', + 'A small status label.', + ); +}); + +afterEach(() => { + fs.rmSync(tmpDir, {recursive: true, force: true}); +}); + +/** @param {any} list @returns {string[]} the integration's listed names */ +function listedNames(list) { + return Object.values(list.data.components) + .flat() + .filter(entry => entry.package === '@acme/widgets') + .map(entry => entry.name) + .sort(); +} + +describe('a component doc that names another export of its file', () => { + it( + "leaves a name a legacy docs package documents to that package's doc", + async () => { + const legacy = path.join(tmpDir, 'node_modules', '@legacy', 'docs'); + fs.mkdirSync(path.join(legacy, 'docs'), {recursive: true}); + fs.writeFileSync( + path.join(legacy, 'package.json'), + JSON.stringify({name: '@legacy/docs', astryx: {docs: './docs'}}), + ); + for (const name of ['AcmeCarousel', 'Slides']) { + fs.writeFileSync( + path.join(legacy, 'docs', `${name}.doc.mjs`), + `export default {type: 'component', name: '${name}', usage: {description: 'Legacy.'}, props: []};\n`, + ); + } + const cwd = tmpDir; + expect((await component('AcmeCarousel', {cwd})).data.package).toBe( + '@legacy/docs', + ); + const scoped = {cwd, package: '@acme/widgets'}; + expect((await component('AcmeCarousel', scoped)).data.package).toBe( + '@acme/widgets', + ); + // Slides.doc.mjs's file name still answers first, as on main. + expect((await component('Slides', {cwd})).data.package).toBe( + '@acme/widgets', + ); + const report = {cwd, category: 'other', reason: 'Needs a size.'}; + for (const name of ['AcmeCarousel', 'acmecarousel']) { + expect((await gapReport(name, report)).data.package).toBe( + '@astryxdesign/core', + ); + } + const hits = (await search('AcmeCarousel', {cwd, type: 'component'})).data + .results; + expect(hits.find(hit => hit.package === '@acme/widgets')?.command).toBe( + 'astryx component AcmeCarousel --package @acme/widgets', + ); + }, + SLOW, + ); + + it( + 'is listed, opened and found by the export name', + async () => { + expect( + listedNames(await component(undefined, {cwd: tmpDir, list: true})), + ).toEqual(['AcmeBadge', 'AcmeCarousel']); + + const detail = await component('AcmeCarousel', {cwd: tmpDir}); + expect(detail.type).toBe('component.detail'); + expect(detail.data).toMatchObject({ + name: 'AcmeCarousel', + package: '@acme/widgets', + import: '@acme/widgets/gallery/Slides', + sourceAvailable: true, + }); + + const hits = ( + await search('AcmeCarousel', {cwd: tmpDir, type: 'component'}) + ).data.results; + expect(hits[0]).toMatchObject({ + name: 'AcmeCarousel', + package: '@acme/widgets', + import: '@acme/widgets/gallery/Slides', + }); + + // The file name opens the same doc, as it did before, routes a gap + // report to the package, and search finds the component by it, a word + // of it included. + expect((await component('Slides', {cwd: tmpDir})).data.name).toBe( + 'AcmeCarousel', + ); + const report = {cwd: tmpDir, category: 'other', reason: 'Needs a size.'}; + expect((await gapReport('Slides', report)).data.package).toBe( + '@acme/widgets', + ); + for (const query of ['Slides', 'slide']) { + const byFileName = ( + await search(query, {cwd: tmpDir, type: 'component'}) + ).data.results; + expect(byFileName[0]?.name).toBe('AcmeCarousel'); + } + }, + SLOW, + ); + + it( + "leaves a Core name to Core unless --package names it, and passes Doctor's package checks", + async () => { + // Overlays.tsx exports Dialog, a Core name, and nothing named Overlays. + writeComponent( + 'gallery/Overlays', + 'Dialog', + 'export function AcmeOverlays() { return null; }\nexport function Dialog() { return null; }\n', + 'Overlays.', + ); + + const validation = await validateIntegration(undefined, {cwd: pkgDir}); + expect(validation.data.issues).toEqual([]); + + const conflicts = await integrationComponentConflicts(undefined, { + cwd: pkgDir, + }); + expect(conflicts.data.conflicts.map(conflict => conflict.name)).toEqual([ + 'Dialog', + ]); + + // Lookups by the bare name still answer with Core's Dialog, as before. + const cwd = tmpDir; + const core = '@astryxdesign/core'; + expect((await component('Dialog', {cwd})).data.package).toBe(core); + expect((await component('Dialog', {cwd, props: true})).package).toBe( + core, + ); + expect((await swizzleCopy('Dialog', {cwd})).package).toBe(core); + const report = {cwd, category: 'other', reason: 'Needs a size.'}; + expect((await gapReport('Dialog', report)).data.package).toBe(core); + // The list and --package reach the integration's Dialog. + const scoped = {cwd, package: '@acme/widgets'}; + expect((await component('Dialog', scoped)).data.import).toBe( + '@acme/widgets/gallery/Overlays', + ); + expect( + listedNames(await component(undefined, {cwd, list: true})), + ).toContain('Dialog'); + // Search shows both; the package's hit names its package and follows + // Core's. + const hits = (await search('Dialog', {cwd, type: 'component'})).data + .results; + const ours = hits.findIndex(hit => hit.package === '@acme/widgets'); + expect(hits[0]).toMatchObject({name: 'Dialog', package: core}); + expect(hits[ours]).toMatchObject({ + name: 'Dialog', + command: 'astryx component Dialog --package @acme/widgets', + }); + }, + SLOW, + ); + + it( + 'leaves a Core name with no folder of its own unknown to swizzle, as before', + async () => { + // Core documents CheckboxIndicator inside Indicator's doc, with no folder. + // BaseTable, named after its file, keeps routing reports to its package. + writeComponent( + 'BaseTable', + 'BaseTable', + 'export function BaseTable() { return null; }\n', + 'A file-named table.', + ); + writeComponent( + 'gallery/Indicator', + 'CheckboxIndicator', + 'export function CheckboxIndicator() { return null; }\n', + 'A header.', + ); + const cwd = tmpDir; + expect((await component('CheckboxIndicator', {cwd})).data.package).toBe( + '@astryxdesign/core', + ); + await expect( + swizzleCopy('CheckboxIndicator', {cwd}), + ).rejects.toMatchObject({ + code: 'ERR_UNKNOWN_COMPONENT', + }); + const scoped = {cwd, package: '@acme/widgets'}; + expect((await swizzleCopy('CheckboxIndicator', scoped)).package).toBe( + '@acme/widgets', + ); + const report = {cwd, category: 'other', reason: 'Needs a size.'}; + expect((await gapReport('CheckboxIndicator', report)).data.package).toBe( + '@astryxdesign/core', + ); + const hit = ( + await search('CheckboxIndicator', {cwd, type: 'component'}) + ).data.results.find(result => result.package === '@acme/widgets'); + expect(hit?.command).toBe( + 'astryx component CheckboxIndicator --package @acme/widgets', + ); + expect((await gapReport('BaseTable', report)).data.package).toBe( + '@acme/widgets', + ); + }, + SLOW, + ); + + it.each([ + // props must be an array: the loader rejects this doc. + [ + 'invalid', + "export default {type: 'component', name: 'AcmeTab', props: 'x'};", + ], + [ + 'hidden', + "export default {\n type: 'component',\n name: 'AcmeTab',\n hidden: true,\n usage: {description: 'x'},\n props: [],\n};", + ], + ])( + "leaves a name to another package's file-named doc even when that doc is %s", + async (kind, doc) => { + writeComponent( + 'gallery/Strip', + 'AcmeTab', + 'export function AcmeTab() { return null; }\n', + 'A strip of tabs.', + ); + const beta = path.join(tmpDir, 'node_modules', '@beta', 'kit'); + fs.mkdirSync(path.join(beta, 'src'), {recursive: true}); + for (const [file, text] of Object.entries({ + 'package.json': '{"name": "@beta/kit", "version": "1.0.0"}', + 'astryx.integration.mjs': "export default {components: './src'};", + 'src/AcmeTab.tsx': 'export function AcmeTab() { return null; }', + 'src/AcmeTab.doc.mjs': doc, + })) { + fs.writeFileSync(path.join(beta, file), `${text}\n`); + } + fs.writeFileSync( + path.join(tmpDir, 'astryx.config.mjs'), + "export default {integrations: ['@acme/widgets', '@beta/kit']};\n", + ); + + const cwd = tmpDir; + expect((await component('AcmeTab', {cwd, source: true})).package).toBe( + '@beta/kit', + ); + if (kind === 'invalid') { + await expect(component('AcmeTab', {cwd})).rejects.toMatchObject({ + code: 'ERR_INVALID_DOC', + }); + } + const report = {cwd, category: 'other', reason: 'Needs a size.'}; + for (const name of ['AcmeTab', 'acmetab']) { + expect((await gapReport(name, report)).data.package).toBe( + '@astryxdesign/core', + ); + } + const hit = ( + await search('AcmeTab', {cwd, type: 'component'}) + ).data.results.find(result => result.package === '@acme/widgets'); + expect(hit?.command).toBe( + 'astryx component AcmeTab --package @acme/widgets', + ); + }, + SLOW, + ); + + it( + 'leaves a file name Core also has to Core in search, as in detail', + async () => { + // Badge.doc.mjs documents AcmeStatusBadge; its file name is Core's. + writeComponent( + 'status/Badge', + 'AcmeStatusBadge', + 'export function AcmeStatusBadge() { return null; }\n', + 'Shows a status.', + ); + const cwd = tmpDir; + const [first] = (await search('Badge', {cwd, type: 'component'})).data + .results; + expect(first).toMatchObject({ + name: 'Badge', + package: '@astryxdesign/core', + }); + expect((await component('Badge', {cwd})).data.package).toBe( + '@astryxdesign/core', + ); + const [own] = (await search('AcmeStatusBadge', {cwd, type: 'component'})) + .data.results; + expect(own).toMatchObject({ + name: 'AcmeStatusBadge', + command: 'astryx component AcmeStatusBadge', + }); + }, + SLOW, + ); + + it( + 'copies a replacement recorded under its export into its file name folder, as before', + async () => { + const manifest = path.join(pkgDir, 'package.json'); + fs.writeFileSync( + manifest, + JSON.stringify({ + ...JSON.parse(fs.readFileSync(manifest, 'utf-8')), + peerDependencies: { + '@astryxdesign/cli': `>=${COMPONENT_REPLACES_CLI}`, + }, + }), + ); + writeComponent( + 'gallery/Slides', + 'AcmeCarousel', + 'export function AcmeCarousel() { return null; }\n', + 'Cycles through a row of slides.', + {replaces: 'Carousel'}, + ); + + const copied = await swizzleCopy('Carousel', {cwd: tmpDir}); + expect(copied.data).toMatchObject({ + package: '@acme/widgets', + outputDir: path.join('components', 'astryx', 'Slides'), + }); + }, + SLOW, + ); +}); diff --git a/packages/cli/api/gap-report/gap-report.mjs b/packages/cli/api/gap-report/gap-report.mjs index dcd3262a960e5..f5b040a99259f 100644 --- a/packages/cli/api/gap-report/gap-report.mjs +++ b/packages/cli/api/gap-report/gap-report.mjs @@ -17,7 +17,17 @@ import {Worker} from 'node:worker_threads'; import {AstryxError} from '../error.mjs'; import {Project} from '../../foundation/config/project.mjs'; -import {CORE_PACKAGE} from '../../foundation/discovery/component-discovery.mjs'; +import { + CORE_PACKAGE, + discoverIntegrationComponents, + docStem, + findComponentReadme, + findIntegrationComponent, + integrationRecordHold, + unscopedComponentOwners, +} from '../../foundation/discovery/component-discovery.mjs'; +import {legacyExternalKeeps} from '../component/_adapter.mjs'; +import {findCoreDir} from '../../foundation/fs/paths.mjs'; import {captureEnv} from '../../foundation/debug/event.mjs'; import {ERROR_CODES} from '../../foundation/response/error-codes.mjs'; import {parseGapReportReceipt} from '../../authoring/gap-report/parse.mjs'; @@ -178,13 +188,65 @@ async function selectTarget(project, component, packageName) { coreName, ); if (replacement) return targetForPackage(project, replacement.package); + // The owner rule runs over every owner discovery finds, loadable or not + // (a doc recorded under another export still answers to its file's name); + // then only owners whose docs load route a report. + const wanted = bareComponent.toLowerCase(); + const loads = new Set(records.map(r => `${r.package}\0${r.docPath}`)); + const recorded = project.loadedIntegrations.flatMap(integration => + discoverIntegrationComponents(integration).flatMap(r => + r.name.toLowerCase() === wanted + ? [{...r, hold: integrationRecordHold(r)}] + : docStem(r.docPath).toLowerCase() === wanted + ? [{...r, hold: 'doc'}] + : [], + ), + ); + // The checks below find files by name, so they run on every spelling the + // case-blind match above found, as well as the one typed. + const spellings = new Set([ + bareComponent, + ...recorded.flatMap(r => + [r.name, docStem(r.docPath)].filter(n => n.toLowerCase() === wanted), + ), + ]); + // Core also holds a name only its doc lookup finds, as in detail: a name a + // doc records yields to it, though it routes no report itself. + const coreDir = findCoreDir(project.cwd); + const coreDocs = + coreDir != null && + [...spellings].some(name => findComponentReadme(coreDir, name) != null); + const ruled = unscopedComponentOwners([ + ...records + .filter( + r => r.package === CORE_PACKAGE && r.name.toLowerCase() === wanted, + ) + .map(r => ({...r, hold: 'core'})), + ...recorded, + // A hidden doc named after its file still holds that name, as the + // component lookup finds it; it routes no report. + ...project.loadedIntegrations.flatMap(integration => + [...spellings].flatMap(spelling => { + const found = findIntegrationComponent(integration, spelling); + return found?.hold === 'file' + ? [{...found, package: integration.name}] + : []; + }), + ), + ]).filter(r => !coreDocs || r.hold !== 'doc'); + // A legacy docs package keeps an export name a doc records, as in detail. const owners = [ ...new Set( - records + ([...spellings].some(name => + legacyExternalKeeps(project.cwd, name, ruled), + ) + ? [] + : ruled + ) .filter( - record => record.name.toLowerCase() === bareComponent.toLowerCase(), + r => r.hold === 'core' || loads.has(`${r.package}\0${r.docPath}`), ) - .map(record => record.package), + .map(r => r.package), ), ]; diff --git a/packages/cli/api/integration/pack-check.mjs b/packages/cli/api/integration/pack-check.mjs index b43456d0aa12f..287778f44c877 100644 --- a/packages/cli/api/integration/pack-check.mjs +++ b/packages/cli/api/integration/pack-check.mjs @@ -51,6 +51,7 @@ import { } from '../theme/build/build.mjs'; import { discoverIntegrationComponents, + docStem, resolveIntegrationImportPath, } from '../../foundation/discovery/component-discovery.mjs'; import {loadComponentReplacements} from '../component/_adapter.mjs'; @@ -480,7 +481,7 @@ async function validatePackedComponentExports(integration, scratchBase) { issues.push( error( 'component_doc_unloadable', - `Component "${record.name}" doc at "${record.docPath}" could not be loaded: ${err instanceof Error ? err.message : String(err)}`, + `Component "${docStem(record.docPath)}" doc at "${record.docPath}" could not be loaded: ${err instanceof Error ? err.message : String(err)}`, ), ); continue; diff --git a/packages/cli/api/integration/pack-check.test.mjs b/packages/cli/api/integration/pack-check.test.mjs index 61b5e3f0101f3..7b65e53396308 100644 --- a/packages/cli/api/integration/pack-check.test.mjs +++ b/packages/cli/api/integration/pack-check.test.mjs @@ -754,6 +754,38 @@ describe('integrationPackCheck', () => { ); }); + it('checks the export a component doc names when its file is named otherwise', async () => { + fs.writeFileSync( + path.join(tmpDir, 'package.json'), + `${JSON.stringify({ + name: '@acme/widgets', + version: '1.0.0', + files: ['astryx.integration.mjs', 'components'], + exports: {'./components/Slides': './components/Slides.tsx'}, + })}\n`, + ); + fs.writeFileSync( + path.join(tmpDir, 'astryx.integration.mjs'), + "export default {components: './components'};\n", + ); + fs.mkdirSync(path.join(tmpDir, 'components')); + // Slides.tsx exports AcmeCarousel and nothing named Slides. + fs.writeFileSync( + path.join(tmpDir, 'components', 'Slides.tsx'), + 'export function AcmeCarousel() { return null; }\n', + ); + fs.writeFileSync( + path.join(tmpDir, 'components', 'Slides.doc.mjs'), + "export default {type: 'component', name: 'AcmeCarousel', import: '@acme/widgets/components/Slides', description: 'x', props: []};\n", + ); + + const result = await integrationPackCheck({cwd: tmpDir}); + expect(result.data.packable).toBe(true); + expect(result.data.contributions.packed.components).toEqual([ + 'AcmeCarousel', + ]); + }); + it('passes a generated component through its exact packed export', async () => { fs.writeFileSync( path.join(tmpDir, 'package.json'), diff --git a/packages/cli/api/search/search.mjs b/packages/cli/api/search/search.mjs index beefeee190467..861a9f87509e2 100644 --- a/packages/cli/api/search/search.mjs +++ b/packages/cli/api/search/search.mjs @@ -81,12 +81,18 @@ import {withListingCache} from '../../foundation/fs/listing-cache.mjs'; import {readDocView} from '../../foundation/doc-compiler/read.mjs'; -import {findCoreDir} from '../../foundation/fs/paths.mjs'; +import { + discoverExternalPackages, + findCoreDir, +} from '../../foundation/fs/paths.mjs'; import { discoverComponents, discoverValidIntegrationComponents, + docStem, familyMember, findComponentReadme, + forEachIntegrationDoc, + integrationRecordHold, resolveImportPath, resolveIntegrationImportPath, CORE_PACKAGE, @@ -98,6 +104,7 @@ import { import { loadComponentReplacements, loadIntegrationsSafely, + resolveLegacyExternalDoc, } from '../component/_adapter.mjs'; import {levenshteinDistance} from '../../foundation/text/string-utils.mjs'; import {discoverTemplates, extractComponents} from '../template/template.mjs'; @@ -150,6 +157,9 @@ import {setResultCoverage} from './coverage.mjs'; * @property {'page'|'block'} [_kind] * @property {string} [_resultName] * @property {string} [_commandName] + * @property {'file'|'doc'} [_hold] - How an integration component holds its + * name, for the owner rule. + * @property {string} [_docPath] - An integration component's doc. */ /** @@ -1110,20 +1120,50 @@ async function gatherCoreComponents(coreDir) { * usage/description from each component's .doc.mjs, same as core. Without * this, an integration component is invisible to `search`/`build` even * though `component --list`/`component ` already resolve it — the two - * discovery paths silently disagreed. + * discovery paths silently disagreed. Also returns, per name, the docs that + * hold it by their file name and the loadable docs that only record it. * @param {import('../../foundation/integrations/integrations.mjs').LoadedIntegration[]} loadedIntegrations - * @returns {Promise} + * @returns {Promise<{candidates: Candidate[], fileNamed: Map>, documented: Map>}>} */ async function gatherIntegrationComponents(loadedIntegrations) { /** @type {Candidate[]} */ const candidates = []; + // What the owner rule reads: the docs that hold a name by their file name, + // loadable or not and hidden ones too, and the loadable docs that only + // record it. + /** @type {Map>} */ + const fileNamed = new Map(); + /** @type {Map>} */ + const documented = new Map(); + /** @param {Map>} map @param {string} name @param {string} docPath */ + const hold = (map, name, docPath) => + map.set(name, (map.get(name) ?? new Set()).add(docPath)); for (const integration of loadedIntegrations) { - const {components} = await discoverValidIntegrationComponents(integration); + const {components, discovered} = + await discoverValidIntegrationComponents(integration); + const renamed = new Set( + discovered + .filter(rec => integrationRecordHold(rec) === 'doc') + .map(rec => rec.docPath), + ); + forEachIntegrationDoc(integration.components, (docPath, stem) => { + if (!renamed.has(docPath)) hold(fileNamed, stem, docPath); + }); for (const rec of components) { const doc = await loadModuleDoc(rec.docPath); + // A doc that names another export of its file still answers to the + // file's name, as `component ` does. + const fileName = docStem(rec.docPath); + if (renamed.has(rec.docPath)) { + hold(documented, rec.name, rec.docPath); + hold(documented, fileName, rec.docPath); + } candidates.push({ domain: 'component', name: rec.name, + aliases: fileName !== rec.name ? [fileName] : [], + _hold: integrationRecordHold(rec), + _docPath: rec.docPath, keywords: doc && Array.isArray(doc.keywords) ? doc.keywords : [], description: doc ? doc.usage?.description || doc.description || '' : '', guidance: guidanceFrom(doc), @@ -1148,7 +1188,7 @@ async function gatherIntegrationComponents(loadedIntegrations) { }); } } - return candidates; + return {candidates, fileNamed, documented}; } /** @@ -1160,11 +1200,12 @@ async function gatherIntegrationComponents(loadedIntegrations) { */ async function gatherComponents(coreDir, cwd) { const loadedIntegrations = await loadIntegrationsSafely(cwd); - const [core, integrations, replacements] = await Promise.all([ + const [core, gathered, replacements] = await Promise.all([ gatherCoreComponents(coreDir), gatherIntegrationComponents(loadedIntegrations), loadComponentReplacements(coreDir, loadedIntegrations), ]); + const {candidates: integrations, fileNamed, documented} = gathered; // An active replacement answers to the Core name it replaces, so a search // for that name finds the replacement and not the Core original // (spec:AST-035 FR11). @@ -1175,9 +1216,43 @@ async function gatherComponents(coreDir, cwd) { active.target, ]), ); + // The owner rule, as component detail applies it: a candidate `component + // ` does not open names its package and follows the owner, and a + // file-name alias that opens another owner is dropped. + const coreNames = new Set(core.map(candidate => candidate.name)); + /** @param {string} name @param {Candidate} candidate */ + const opens = (name, candidate) => + !coreNames.has(name) && + findComponentReadme(coreDir, name) == null && + [...(fileNamed.get(name) ?? [])].every(doc => doc === candidate._docPath) && + [...(documented.get(name) ?? [])].every(doc => doc === candidate._docPath); + // A legacy docs package keeps an exact name over an export name a doc records. + /** @type {ReturnType | undefined} */ + let externals; + /** @param {string} name */ + const legacyDocuments = name => + (externals ??= discoverExternalPackages(cwd)).some( + ext => resolveLegacyExternalDoc(ext, name) != null, + ); + const ruled = integrations.map(candidate => { + const aliases = (candidate.aliases ?? []).filter(alias => + opens(alias, candidate), + ); + return candidate._hold === 'doc' && + (!opens(candidate.name, candidate) || legacyDocuments(candidate.name)) + ? { + ...candidate, + aliases, + _command: `astryx component ${candidate.name} --package ${candidate._package}`, + } + : {...candidate, aliases}; + }); return [ ...core.filter(candidate => !replacements.forTarget(candidate.name)), - ...integrations.map(candidate => { + ...[ + ...ruled.filter(candidate => !candidate._command), + ...ruled.filter(candidate => candidate._command), + ].map(candidate => { const target = targetOf.get(`${candidate._package}\0${candidate.name}`); if (target != null) { return {...candidate, aliases: [...(candidate.aliases ?? []), target]}; diff --git a/packages/cli/api/swizzle/copy/copy.mjs b/packages/cli/api/swizzle/copy/copy.mjs index 785c90fac190b..3e7adda44acec 100644 --- a/packages/cli/api/swizzle/copy/copy.mjs +++ b/packages/cli/api/swizzle/copy/copy.mjs @@ -21,8 +21,10 @@ import {checkGhCli} from '../_github.mjs'; import {Project} from '../../../foundation/config/project.mjs'; import { CORE_PACKAGE, - findIntegrationComponentDoc, - findIntegrationComponentSource, + docStem, + findComponentReadme, + findIntegrationComponent, + unscopedComponentOwners, } from '../../../foundation/discovery/component-discovery.mjs'; import {ERROR_CODES} from '../../../foundation/response/error-codes.mjs'; import {AstryxError, writeFailed} from '../../error.mjs'; @@ -126,33 +128,38 @@ async function loadConfigSafely(cwd) { /** * Build the set of OWNER packages that provide a component named `name` across - * core + every loaded integration. + * core + every loaded integration. Core holds every name it documents, but has + * source to copy (`sourceDir`) only for a name with its own folder. * @param {string} coreDir * @param {Array<{name: string, components?: string, issuesUrl?: string}>} loadedIntegrations * @param {string} name * @param {string|undefined} coreIssuesUrl - * @returns {Array<{package: string, sourceDir: string|null, ownerPackage: string, issuesUrl: string|undefined}>} + * @returns {Array<{package: string, sourceDir: string|null, docPath: string|null, ownerPackage: string, issuesUrl: string|undefined, hold: string}>} */ function resolveOwners(coreDir, loadedIntegrations, name, coreIssuesUrl) { const owners = []; const coreComponentDir = path.join(coreDir, 'src', name); - if (fs.existsSync(coreComponentDir)) { + const coreFolder = fs.existsSync(coreComponentDir); + if (coreFolder || findComponentReadme(coreDir, name)) { owners.push({ package: CORE_PACKAGE, - sourceDir: coreComponentDir, + sourceDir: coreFolder ? coreComponentDir : null, + docPath: null, ownerPackage: CORE_PACKAGE, issuesUrl: coreIssuesUrl || DEFAULT_ISSUES_URL, + hold: 'core', }); } for (const integration of loadedIntegrations) { - const docPath = findIntegrationComponentDoc(integration, name); - if (!docPath) continue; - const sourcePath = findIntegrationComponentSource(integration, name); + const found = findIntegrationComponent(integration, name); + if (!found) continue; owners.push({ package: integration.name, - sourceDir: sourcePath ? path.dirname(sourcePath) : null, + sourceDir: found.sourcePath ? path.dirname(found.sourcePath) : null, + docPath: found.docPath, ownerPackage: integration.name, issuesUrl: integration.issuesUrl, + hold: found.hold, }); } return owners; @@ -222,25 +229,32 @@ export async function swizzleCopy(component, options = {}) { : undefined; // An active integration replacement answers to the Core name it replaces, // bare or qualified by its own package (spec:AST-035 FR11): copy it under its - // own name. `--package @astryxdesign/core` still copies the Core original. + // doc's file name, the folder it has always gone to. `--package + // @astryxdesign/core` still copies the Core original. if (pkg !== CORE_PACKAGE) { const replacement = await resolveComponentReplacement( coreDir, loadedIntegrations, dirName, ); + const copyName = replacement ? docStem(replacement.docPath) : null; if ( replacement && - (!pkg || (pkg === replacement.package && replacement.name !== dirName)) + copyName && + (!pkg || (pkg === replacement.package && copyName !== dirName)) ) { - return swizzleCopy(replacement.name, { + return swizzleCopy(copyName, { ...options, package: replacement.package, }); } } - const allOwners = resolveOwners(coreDir, loadedIntegrations, dirName, coreIssuesUrl); + const found = resolveOwners(coreDir, loadedIntegrations, dirName, coreIssuesUrl); + // The owner rule, then a Core name without its own folder drops out. + const allOwners = (pkg ? found : unscopedComponentOwners(found)).filter( + owner => owner.package !== CORE_PACKAGE || owner.sourceDir != null, + ); if (allOwners.length === 0) { throw new AstryxError( diff --git a/packages/cli/assets/docs/tree/component-imports.doc.mjs b/packages/cli/assets/docs/tree/component-imports.doc.mjs index 2c173db02b21c..023a1ec87b7e0 100644 --- a/packages/cli/assets/docs/tree/component-imports.doc.mjs +++ b/packages/cli/assets/docs/tree/component-imports.doc.mjs @@ -35,6 +35,14 @@ export const docs = { type: 'prose', text: 'Add writes the entry only when package.json already has an `exports` map, so start every package with `"exports": {}`.', }, + { + type: 'prose', + text: 'A file does not have to be named after its component. When `Slides.tsx` exports `AcmeCarousel` and nothing named `Slides`, keep the doc beside it as `Slides.doc.mjs` and set its `name` to `AcmeCarousel`: apps list, search and open the component by that name, and `integration verify` checks that its import exports `AcmeCarousel`. A file that exports its own name keeps that name.', + }, + { + type: 'prose', + text: "The file name still opens the doc. A name that Core or another doc's file already has stays theirs, and so does a Core name your package replaces: with `Badge.doc.mjs` naming `AcmeBadge`, `astryx component Badge` opens Core's `Badge`, and `--package` with your package name reaches yours.", + }, ], }, { diff --git a/packages/cli/authoring/doctypes/component/component.doc.mjs b/packages/cli/authoring/doctypes/component/component.doc.mjs index 5e37b5e1a211a..5cd3059cbac65 100644 --- a/packages/cli/authoring/doctypes/component/component.doc.mjs +++ b/packages/cli/authoring/doctypes/component/component.doc.mjs @@ -30,7 +30,7 @@ export const doc = { name: 'name', type: 'string', description: - "Stable machine identity and directory name without the Astryx prefix, PascalCase. e.g. 'Button', 'TextInput', 'AppShell'. Change `displayName`, not `name`, to edit the visible label; registry URLs derive from this identity by default.", + "Stable machine identity and directory name without the Astryx prefix, PascalCase. e.g. 'Button', 'TextInput', 'AppShell'. For an integration component it is the export apps import: the doc sits beside the file that exports it, and when that file exports nothing under its own name, `name` may name the export it does have. Change `displayName`, not `name`, to edit the visible label; registry URLs derive from this identity by default.", required: true, }, { diff --git a/packages/cli/authoring/doctypes/component/type.ts b/packages/cli/authoring/doctypes/component/type.ts index e99d63a115e8d..7b131b60eec80 100644 --- a/packages/cli/authoring/doctypes/component/type.ts +++ b/packages/cli/authoring/doctypes/component/type.ts @@ -33,9 +33,12 @@ export interface ComponentBaseDoc extends AuthoredDocGraphFields { type?: 'component'; /** * Stable machine identity and directory name without the Astryx prefix, - * PascalCase (for example `Button`, `Table`, or `AppShell`). Do not change - * this to edit the visible label; use `displayName` for that. Registry URLs - * derive from this value by default. + * PascalCase (for example `Button`, `Table`, or `AppShell`). For an + * integration component it is the export apps import: the doc sits beside + * the file that exports it, and when that file exports nothing under its own + * name, `name` may name the export it does have. Do not change this to edit + * the visible label; use `displayName` for that. Registry URLs derive from + * this value by default. */ name: string; /** Human-readable display name with spaces between words, used by the diff --git a/packages/cli/foundation/discovery/component-discovery.export-names.test.mjs b/packages/cli/foundation/discovery/component-discovery.export-names.test.mjs new file mode 100644 index 0000000000000..479a5816c7336 --- /dev/null +++ b/packages/cli/foundation/discovery/component-discovery.export-names.test.mjs @@ -0,0 +1,293 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Integration components are found by the names their docs document; + * every doc that works today reads exactly as before. + */ + +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + discoverIntegrationComponents, + discoverValidIntegrationComponents, + findIntegrationComponent, + findIntegrationComponentDoc, + findIntegrationComponentSource, + unscopedComponentOwners, +} from './component-discovery.mjs'; + +/** @type {string} the integration's components root */ +let root; + +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-export-names-')); +}); + +afterEach(() => { + fs.rmSync(root, {recursive: true, force: true}); +}); + +/** + * Write a component doc, and its same-stem source when given. + * @param {string} file path under the root, without extension + * @param {{name: string, source?: string, form?: 'default' | 'docs', suffix?: string, extra?: string}} options + * `name` is written as a JavaScript expression + */ +function writeComponent( + file, + {name, source, form = 'default', suffix = '.doc.mjs', extra = ''}, +) { + const target = path.join(root, file); + fs.mkdirSync(path.dirname(target), {recursive: true}); + const doc = `{\n type: 'component',\n name: ${name},\n usage: {description: 'A component.'},\n props: [],${extra}\n}`; + fs.writeFileSync( + `${target}${suffix}`, + form === 'default' + ? `export default ${doc};\n` + : `export const docs = ${doc};\n`, + ); + if (source != null) fs.writeFileSync(`${target}.tsx`, source); +} + +const integration = () => ({name: '@acme/widgets', components: root}); + +/** @returns {string[]} */ +const names = () => + discoverIntegrationComponents(integration()) + .map(record => record.name) + .sort(); + +describe('integration components whose doc names another export of its file', () => { + it('records a doc beside a kebab-case file under the export its doc names', async () => { + writeComponent('artifact-viewer', { + name: "'AcmeArtifactViewer'", + source: 'export function AcmeArtifactViewer() { return null; }\n', + }); + + const [record] = discoverIntegrationComponents(integration()); + expect(record).toMatchObject({ + name: 'AcmeArtifactViewer', + package: '@acme/widgets', + docPath: path.join(root, 'artifact-viewer.doc.mjs'), + sourcePath: path.join(root, 'artifact-viewer.tsx'), + }); + expect( + (await discoverValidIntegrationComponents(integration())).components, + ).toEqual([expect.objectContaining({name: 'AcmeArtifactViewer'})]); + expect( + findIntegrationComponentDoc(integration(), 'AcmeArtifactViewer'), + ).toBe(record.docPath); + expect(findIntegrationComponentDoc(integration(), 'artifact-viewer')).toBe( + record.docPath, + ); + expect( + findIntegrationComponentSource(integration(), 'AcmeArtifactViewer'), + ).toBe(record.sourcePath); + }); + + it('leaves a name to Core or a file-named component over one a doc records', () => { + const core = {package: '@astryxdesign/core', hold: 'core'}; + const fileNamed = {package: '@beta/kit', hold: 'file'}; + const documented = {package: '@acme/widgets', hold: 'doc'}; + expect(unscopedComponentOwners([documented, fileNamed])).toEqual([ + fileNamed, + ]); + expect(unscopedComponentOwners([core, documented])).toEqual([core]); + expect(unscopedComponentOwners([documented])).toEqual([documented]); + + // Badge.doc.mjs records AcmeBadge, so the doc holds both names only + // because it documents them; Panel.doc.mjs names an export another file + // has, so it keeps its file name. + writeComponent('Badge', { + name: "'AcmeBadge'", + source: 'export function AcmeBadge() { return null; }\n', + }); + writeComponent('Panel', { + name: "'AcmeChip'", + source: 'export function AcmeChip() { return null; }\n', + }); + writeComponent('AcmeChip', { + name: "'AcmeChip'", + source: 'export function AcmeChip() { return null; }\n', + }); + const hold = (/** @type {string} */ name) => + findIntegrationComponent(integration(), name)?.hold; + expect(hold('AcmeBadge')).toBe('doc'); + expect(hold('Badge')).toBe('doc'); + expect(hold('Panel')).toBe('file'); + expect(hold('AcmeChip')).toBe('file'); + }); + + it('takes no name a hidden doc or a replacement in the package claims', () => { + // A hidden doc is still the file named AcmeCarousel: its name stays its own. + writeComponent('AcmeCarousel', { + name: "'AcmeCarousel'", + extra: '\n hidden: true,', + source: 'export function AcmeCarousel() { return null; }\n', + }); + writeComponent('gallery/Slides', { + name: "'AcmeCarousel'", + source: 'export function AcmeCarousel() { return null; }\n', + }); + // A doc in the package replaces Carousel, so that name answers with it. + writeComponent('Pager', { + name: "'AcmePager'", + extra: "\n replaces: 'Carousel',", + source: 'export function AcmePager() { return null; }\n', + }); + writeComponent('Kit', { + name: "'Carousel'", + source: 'export function Carousel() { return null; }\n', + }); + + expect(names()).toEqual(['AcmePager', 'Kit', 'Slides']); + expect(findIntegrationComponentDoc(integration(), 'AcmeCarousel')).toBe( + path.join(root, 'AcmeCarousel.doc.mjs'), + ); + expect(findIntegrationComponentDoc(integration(), 'Carousel')).toBeNull(); + }); + + it('settles a name two docs document by path, the same in every locale', () => { + // By UTF-16 code unit `B/` sorts before `a/`; a locale's collation puts + // `a/` first. + for (const [file, stem] of [ + ['a/Alpha', 'Alpha'], + ['B/Beta', 'Beta'], + ]) { + writeComponent(file, { + name: "'Twin'", + source: `export function Twin() { return null; }\n// ${stem}\n`, + }); + } + + expect(names()).toEqual(['Alpha', 'Twin']); + expect(findIntegrationComponentDoc(integration(), 'Twin')).toBe( + path.join(root, 'B', 'Beta.doc.mjs'), + ); + }); + + it('reads a name a spread can override, and reports an unreadable doc under its file name', async () => { + fs.writeFileSync( + path.join(root, 'Based.doc.mjs'), + "const base = {\n name: 'Based',\n type: 'component',\n usage: {description: 'x'},\n props: [],\n};\nexport default {...base, name: 'AcmeBased'};\n", + ); + fs.writeFileSync( + path.join(root, 'Based.tsx'), + 'export function AcmeBased() { return null; }\n', + ); + writeComponent('Throws', { + name: "'AcmeThrows'", + source: 'export function AcmeThrows() { return null; }\n', + }); + const throws = path.join(root, 'Throws.doc.mjs'); + fs.writeFileSync( + throws, + `throw new Error('boom');\n${fs.readFileSync(throws, 'utf-8')}`, + ); + + expect(names()).toEqual(['AcmeBased', 'AcmeThrows']); + const {errors} = await discoverValidIntegrationComponents(integration()); + expect(errors).toEqual([ + { + name: 'Throws', + message: 'Component "Throws" has invalid metadata: boom', + }, + ]); + }); + + it('reads a legacy export, a .doc.ts doc and a re-exported component the same way', () => { + writeComponent('Slides', { + name: "'AcmeCarousel'", + form: 'docs', + source: "export {AcmeCarousel} from './carousel/impl';\n", + }); + fs.mkdirSync(path.join(root, 'carousel')); + fs.writeFileSync( + path.join(root, 'carousel', 'impl.tsx'), + 'export function AcmeCarousel() { return null; }\n', + ); + writeComponent('Pager', { + name: "'AcmePager'", + suffix: '.doc.ts', + source: 'export const AcmePager = () => null;\n', + }); + + expect(names()).toEqual(['AcmeCarousel', 'AcmePager']); + }); + + it('keeps the file name for every doc that reads as before', async () => { + // Named after its file. + writeComponent('AcmeBadge', { + name: "'AcmeBadge'", + source: 'export function AcmeBadge() { return null; }\n', + }); + // The file exports its own name, so that name stays. + writeComponent('Chip', { + name: "'AcmeChip'", + source: + 'export function Chip() { return null; }\nexport function AcmeChip() { return null; }\n', + }); + // The doc names an export the file does not have. + writeComponent('Menu', { + name: "'AcmeMenu'", + source: 'export function Dropdown() { return null; }\n', + }); + // A type is not a runtime export. + writeComponent('Shape', { + name: "'AcmeShape'", + source: 'export type AcmeShape = {id: string};\n', + }); + // A name the doc computes is not read. + writeComponent('Tile', { + name: "`${'Acme'}Tile`", + source: 'export function AcmeTile() { return null; }\n', + }); + // Another file already has the name. + writeComponent('Panel', { + name: "'AcmeBadge'", + source: 'export function AcmeBadge() { return null; }\n', + }); + // No source beside it. + writeComponent('Orphan', {name: "'AcmeOrphan'"}); + + expect(names()).toEqual([ + 'AcmeBadge', + 'Chip', + 'Menu', + 'Orphan', + 'Panel', + 'Shape', + 'Tile', + ]); + for (const name of [ + 'AcmeChip', + 'AcmeMenu', + 'AcmeShape', + 'AcmeTile', + 'AcmeOrphan', + ]) { + expect(findIntegrationComponentDoc(integration(), name)).toBeNull(); + } + expect( + (await discoverValidIntegrationComponents(integration())).errors, + ).toEqual([ + { + name: 'Orphan', + message: + 'Component "Orphan" is missing its same-stem source file Orphan.tsx.', + }, + ]); + }); + + it('still leaves a hidden doc out', () => { + writeComponent('Slides', { + name: "'AcmeCarousel'", + extra: '\n hidden: true,', + source: 'export function AcmeCarousel() { return null; }\n', + }); + + expect(names()).toEqual([]); + }); +}); diff --git a/packages/cli/foundation/discovery/component-discovery.mjs b/packages/cli/foundation/discovery/component-discovery.mjs index da4977c41447a..e85004b0dca1c 100644 --- a/packages/cli/foundation/discovery/component-discovery.mjs +++ b/packages/cli/foundation/discovery/component-discovery.mjs @@ -7,10 +7,12 @@ import * as fs from 'node:fs'; import {createRequire} from 'node:module'; import * as path from 'node:path'; +import {DOC_EXPORTS} from '../doc-compiler/read.mjs'; import {listDir, listDirEntries, pathExists} from '../fs/listing-cache.mjs'; import {existsCaseExact} from '../fs/paths.mjs'; import {CORE_PROVIDER_ID} from '../identity/providers.mjs'; import {loadComponentDoc} from './component-loader.mjs'; +import {moduleExportsName, staticExportObject} from './module-exports.mjs'; const require = createRequire(import.meta.url); @@ -82,12 +84,35 @@ function componentNameFromFile(fileName) { const GROUP_RE = /(?:^|\n) {0,2}group:\s*['"]([^'"]+)['"]/; const HIDDEN_COMPONENTS_RE = /(?:^|\n) {0,2}hiddenComponents:\s*\[([^\]]*)\]/; const HIDDEN_RE = /(?:^|\n) {0,2}hidden:\s*true/; +// Every top-level `name: '...'` (or `"name": "..."`) in the file: a quick check +// that a doc is named after its file before anything parses it. +const NAME_FIELD_RE = /(?:^|\n) {0,2}['"]?name['"]?\s*:\s*['"]([^'"\n]+)['"]/g; +// A top-level `replaces: '...'`: the Core name a doc claims for its package. +const REPLACES_FIELD_RE = + /(?:^|\n) {0,2}['"]?replaces['"]?\s*:\s*['"]([^'"\n]+)['"]/; + +/** + * What a quick scan of a doc's text finds before anything parses it: the + * top-level `name` values it writes (none when a spread could set the name) + * and the Core name it replaces. + * @param {string} text + * @returns {{names: string[], replaces: string | null}} + */ +function scanDocText(text) { + return { + names: text.includes('...') + ? [] + : [...text.matchAll(NAME_FIELD_RE)].map(match => match[1]), + replaces: REPLACES_FIELD_RE.exec(text)?.[1] ?? null, + }; +} /** * Read the `group`, `hiddenComponents`, and `hidden` fields from a - * component's .doc.mjs file (synchronous). + * component's .doc.mjs file (synchronous), with what {@link scanDocText} + * finds in it. * @param {string} docPath - * @returns {{group: string | null, hiddenComponents: Set, hidden: boolean}} + * @returns {{group: string | null, hiddenComponents: Set, hidden: boolean, names: string[], replaces: string | null}} */ function readDocMeta(docPath) { try { @@ -106,9 +131,16 @@ function readDocMeta(docPath) { group: groupMatch ? groupMatch[1] : null, hiddenComponents: hiddenSet, hidden, + ...scanDocText(content), }; } catch { - return {group: null, hiddenComponents: new Set(), hidden: false}; + return { + group: null, + hiddenComponents: new Set(), + hidden: false, + names: [], + replaces: null, + }; } } @@ -843,6 +875,95 @@ export function findExternalComponentDoc(docsDir, name) { // commands — and the future integration-component swizzle — can disambiguate // by package and route source/issues correctly. +// A doc's `name` can name another export of its file; every other doc reads as +// before. +const COMPONENT_NAME_RE = /^[A-Z][\w$]*$/; + +/** @type {Map | null}>} doc reads, reused while the doc file is unchanged */ +const docReads = new Map(); + +/** + * A file's identity for a cache: ctime and inode change on a rewrite even when + * npm stamps the same size and mtime. + * @param {string} file + */ +function fileStamp(file) { + try { + const {size, mtimeMs, ctimeMs, ino} = fs.statSync(file); + return `${size}:${mtimeMs}:${ctimeMs}:${ino}`; + } catch { + return 'missing'; + } +} + +/** UTF-16 code-unit order, the same in every locale. @param {string} a @param {string} b */ +function byCodeUnit(a, b) { + return a < b ? -1 : a > b ? 1 : 0; +} + +/** + * A component doc's exported object as plain data, read without running it. + * @param {string} docPath + * @returns {Record | null} + */ +function docObject(docPath) { + const stamp = fileStamp(docPath); + const cached = docReads.get(docPath); + if (cached?.stamp === stamp) return cached.value; + const value = staticExportObject(docPath, DOC_EXPORTS.components); + docReads.set(docPath, {stamp, value}); + return value; +} + +/** @param {string} docPath */ +export function docStem(docPath) { + return path.basename(docPath).replace(/\.doc\.(ts|mjs|js)$/, ''); +} + +/** + * The export a component doc names when its same-stem source is not named + * after an export, or null to keep the stem. + * @param {{docPath: string, sourcePath: string|null, stem: string, names: string[]}} doc + * `names`: top-level names a quick scan found (none: the doc is read) + * @returns {string | null} + */ +function otherExportName({docPath, sourcePath, stem, names}) { + if (sourcePath == null) return null; + if (names.length > 0 && names.every(name => name === stem)) return null; + const name = docObject(docPath)?.name; + return typeof name === 'string' && + name !== stem && + COMPONENT_NAME_RE.test(name) && + moduleExportsName(sourcePath, name) && + !moduleExportsName(sourcePath, stem) + ? name + : null; +} + +/** + * Visit every component doc under an integration's components root, hidden + * ones included, in discovery order, with its stem. + * @param {string | undefined} componentsDir + * @param {(docPath: string, stem: string) => void} visit + */ +export function forEachIntegrationDoc(componentsDir, visit) { + if (!componentsDir || !pathExists(componentsDir)) return; + /** @param {string} dirPath */ + function scanDir(dirPath) { + for (const entry of listDirEntries(dirPath)) { + if (entry.name === 'node_modules' || entry.name === '__tests__') continue; + const fullPath = path.join(dirPath, entry.name); + if (entry.isDirectory()) { + scanDir(fullPath); + continue; + } + const suffix = INTEGRATION_DOC_SUFFIXES.find(s => entry.name.endsWith(s)); + if (suffix) visit(fullPath, entry.name.slice(0, -suffix.length)); + } + } + scanDir(componentsDir); +} + /** * Given an integration component doc path, return the sibling component source * (`{Name}.tsx`) if one exists, else null. @@ -860,49 +981,71 @@ function integrationSourceForDoc(docPath) { /** * Discover ownership records for the components contributed by a single loaded * integration. Scans the integration's resolved `components` dir for same-stem - * doc files and records each with owner package + issuesUrl + sourcePath. + * doc files and records each with owner package + issuesUrl + sourcePath. A doc + * that names a differently named export of its file is recorded under that + * export unless the name is taken: every doc's file name (a hidden doc's too), + * a Core name a doc in the package replaces, and a name an earlier doc records. * * @param {{name: string, components?: string, issuesUrl?: string}} integration * a single entry from `loadedIntegrations` (lib/integrations.mjs) + * @param {{documentedNames?: boolean}} [options] `documentedNames: false` + * records each doc under its file name alone, without reading the names + * docs document: for a caller that needs only what each doc says * @returns {Array<{name: string, package: string, docPath: string, sourcePath: string|null, issuesUrl: string|undefined, group: string|null}>} */ -export function discoverIntegrationComponents(integration) { +export function discoverIntegrationComponents( + integration, + {documentedNames = true} = {}, +) { const componentsDir = integration?.components; if (!componentsDir || !pathExists(componentsDir)) return []; /** @type {Map} */ const byName = new Map(); - - /** @param {string} dirPath */ - function scanDir(dirPath) { - const entries = listDirEntries(dirPath); - for (const entry of entries) { - if (entry.name === 'node_modules' || entry.name === '__tests__') continue; - const fullPath = path.join(dirPath, entry.name); - if (entry.isDirectory()) { - scanDir(fullPath); - continue; - } - const suffix = INTEGRATION_DOC_SUFFIXES.find(s => entry.name.endsWith(s)); - if (!suffix) continue; - const name = entry.name.slice(0, -suffix.length); - const {group, hidden} = readDocMeta(fullPath); - if (hidden) continue; - // First doc wins per name (precedence matches INTEGRATION_DOC_SUFFIXES). - if (byName.has(name)) continue; - byName.set(name, { - name, - package: integration.name, - docPath: fullPath, - sourcePath: integrationSourceForDoc(fullPath), - issuesUrl: integration.issuesUrl, - group: group ?? null, - }); - } + /** @type {Map} stem → quick scan */ + const scans = new Map(); + /** @type {Set} names no doc can record under another name */ + const taken = new Set(); + + forEachIntegrationDoc(componentsDir, (fullPath, name) => { + taken.add(name); + const {group, hidden, names, replaces} = readDocMeta(fullPath); + if (hidden) return; + // First doc wins per name (precedence matches INTEGRATION_DOC_SUFFIXES). + if (byName.has(name)) return; + byName.set(name, { + name, + package: integration.name, + docPath: fullPath, + sourcePath: integrationSourceForDoc(fullPath), + issuesUrl: integration.issuesUrl, + group: group ?? null, + }); + scans.set(name, {names, replaces}); + }); + if (!documentedNames) return [...byName.values()]; + + // A doc that names a differently named export of its file is listed under + // that export, unless the name is taken. + for (const {replaces} of scans.values()) if (replaces) taken.add(replaces); + const records = [...byName.values()]; + const byPath = [...records].sort((a, b) => byCodeUnit(a.docPath, b.docPath)); + /** @type {Map} stem → export name */ + const exportNames = new Map(); + for (const record of byPath) { + const exportName = otherExportName({ + ...record, + stem: record.name, + names: scans.get(record.name)?.names ?? [], + }); + if (exportName == null || taken.has(exportName)) continue; + taken.add(exportName); + exportNames.set(record.name, exportName); } - - scanDir(componentsDir); - return [...byName.values()]; + return records.map(record => { + const name = exportNames.get(record.name); + return name ? {...record, name} : record; + }); } /** @@ -910,17 +1053,20 @@ export function discoverIntegrationComponents(integration) { * Invalid metadata or a missing same-stem source removes only that component; * callers can report the returned errors while retaining valid siblings. A * component whose doc sets `replaces` carries the value as written; the - * component replacement resolver decides what it means. + * component replacement resolver decides what it means. A doc that fails to + * load is reported under its file name: what it says cannot be trusted. * * @param {{name: string, components?: string, issuesUrl?: string}} integration + * @param {{documentedNames?: boolean}} [options] as for + * {@link discoverIntegrationComponents} * @returns {Promise<{ * components: Array<{name: string, package: string, docPath: string, sourcePath: string, issuesUrl: string|undefined, group: string|null, replaces?: unknown}>, * discovered: Array<{name: string, package: string, docPath: string, sourcePath: string|null, issuesUrl: string|undefined, group: string|null}>, * errors: Array<{name: string, message: string}>, * }>} */ -export async function discoverValidIntegrationComponents(integration) { - const discovered = discoverIntegrationComponents(integration); +export async function discoverValidIntegrationComponents(integration, options) { + const discovered = discoverIntegrationComponents(integration, options); /** @type {Array<{name: string, package: string, docPath: string, sourcePath: string, issuesUrl: string|undefined, group: string|null, replaces?: unknown}>} */ const components = []; /** @type {Array<{name: string, message: string}>} */ @@ -946,9 +1092,10 @@ export async function discoverValidIntegrationComponents(integration) { : valid, ); } catch (err) { + const name = docStem(record.docPath); errors.push({ - name: record.name, - message: `Component "${record.name}" has invalid metadata: ${ + name, + message: `Component "${name}" has invalid metadata: ${ err instanceof Error ? err.message : String(err) }`, }); @@ -958,19 +1105,122 @@ export async function discoverValidIntegrationComponents(integration) { return {components, discovered, errors}; } +/** + * How an owner holds a component name, which the owner rule + * ({@link unscopedComponentOwners}) reads: `core` when Core documents it; + * `file` when an integration doc named after its file is recorded under it; + * `doc` when only a doc records it: an export name, or the file name of a doc + * recorded under another export. + * @typedef {'core' | 'file' | 'doc'} NameHold + */ + +/** + * How a discovered integration record holds its name. + * @param {{name: string, docPath: string|null}} record + * @returns {'file' | 'doc'} + */ +export function integrationRecordHold(record) { + return record.docPath == null || docStem(record.docPath) === record.name + ? 'file' + : 'doc'; +} + +/** + * The doc that records `name` though its file is named otherwise, with its + * source, reading only docs that write the name as a string. + * + * @param {string} componentsDir + * @param {string} name + * @returns {{docPath: string, sourcePath: string|null} | null} + */ +function findDocumentedComponent(componentsDir, name) { + if (!COMPONENT_NAME_RE.test(name)) return null; + const literals = [`'${name}'`, `"${name}"`, `\`${name}\``]; + /** @type {Set} */ + const stems = new Set(); + /** @type {Array<{docPath: string, stem: string, names: string[], replaces: string | null, sourcePath: string|null}>} */ + const candidates = []; + forEachIntegrationDoc(componentsDir, (docPath, stem) => { + let text; + try { + text = fs.readFileSync(docPath, 'utf-8'); + } catch { + return; + } + // The docs discovery records: the first visible doc for each stem. + if (HIDDEN_RE.test(text) || stems.has(stem)) return; + stems.add(stem); + if (!literals.some(literal => text.includes(literal))) return; + candidates.push({ + docPath, + stem, + ...scanDocText(text), + sourcePath: integrationSourceForDoc(docPath), + }); + }); + // A Core name a doc in the package replaces is the replacement's to answer. + if (candidates.some(candidate => candidate.replaces === name)) return null; + candidates.sort((a, b) => byCodeUnit(a.docPath, b.docPath)); + const found = candidates.find( + candidate => otherExportName(candidate) === name, + ); + return found ? {docPath: found.docPath, sourcePath: found.sourcePath} : null; +} + +/** + * The integration component a name finds, as discovery records it, and its + * hold: the same-stem doc first, then a doc that records the name. + * + * @param {{components?: string}} integration + * @param {string} name + * @returns {{docPath: string, sourcePath: string|null, hold: 'file' | 'doc'} | null} + */ +export function findIntegrationComponent(integration, name) { + const componentsDir = integration?.components; + if (!componentsDir || !pathExists(componentsDir)) return null; + const sameStem = findSameStemDoc(componentsDir, name); + if (sameStem) { + const exportName = otherExportName({ + docPath: sameStem, + sourcePath: integrationSourceForDoc(sameStem), + stem: name, + names: readDocMeta(sameStem).names, + }); + const renamed = + exportName != null && + findSameStemDoc(componentsDir, exportName) == null && + findDocumentedComponent(componentsDir, exportName)?.docPath === sameStem; + return { + docPath: sameStem, + sourcePath: integrationSourceForDoc(sameStem), + hold: renamed ? 'doc' : 'file', + }; + } + const found = findDocumentedComponent(componentsDir, name); + return found && {...found, hold: 'doc'}; +} + /** * Find an integration component's doc file by name within a loaded * integration's resolved `components` dir. Honors the same-stem convention - * (`{Name}.doc.{ts,mjs,js}`), preferring `.ts` → `.mjs` → `.js`. + * (`{Name}.doc.{ts,mjs,js}`), preferring `.ts` → `.mjs` → `.js`, and finds a + * doc that records the component under a differently named export of its file. * * @param {{components?: string}} integration * @param {string} name bare component name (no `XDS`/`Astryx` prefix) * @returns {string|null} */ export function findIntegrationComponentDoc(integration, name) { - const componentsDir = integration?.components; - if (!componentsDir || !pathExists(componentsDir)) return null; + return findIntegrationComponent(integration, name)?.docPath ?? null; +} +/** + * The first `{Name}.doc.{ts,mjs,js}` under `componentsDir`, shallowest first. + * @param {string} componentsDir + * @param {string} name + * @returns {string|null} + */ +function findSameStemDoc(componentsDir, name) { /** * @param {string} dirPath * @returns {string | null} @@ -996,18 +1246,32 @@ export function findIntegrationComponentDoc(integration, name) { } /** - * Find an integration component's swizzleable source file (`{Name}.tsx`) by - * name within a loaded integration's resolved `components` dir. Returns null - * when the integration ships docs without source. + * Find an integration component's swizzleable source file by name within a + * loaded integration's resolved `components` dir: the same-stem `{Name}.tsx` + * of its doc. Returns null when the integration ships docs without source. * * @param {{components?: string}} integration * @param {string} name bare component name * @returns {string|null} */ export function findIntegrationComponentSource(integration, name) { - const docPath = findIntegrationComponentDoc(integration, name); - if (!docPath) return null; - return integrationSourceForDoc(docPath); + return findIntegrationComponent(integration, name)?.sourcePath ?? null; +} + +/** + * The owner rule: an owner that holds a name only because a doc records it + * (`hold: 'doc'`) yields to Core and to every file-named owner; `--package` + * still reaches it. Surfaces resolve a replacement first, then this rule, then + * which docs load. + * + * @template {{hold?: string}} T + * @param {T[]} owners each with its {@link NameHold}; an owner without one + * counts as Core's or file-named + * @returns {T[]} + */ +export function unscopedComponentOwners(owners) { + const primary = owners.filter(owner => owner.hold !== 'doc'); + return primary.length > 0 ? primary : owners; } /** diff --git a/packages/cli/foundation/discovery/component-replacement.mjs b/packages/cli/foundation/discovery/component-replacement.mjs index 20de30ff52847..9ad45968a7969 100644 --- a/packages/cli/foundation/discovery/component-replacement.mjs +++ b/packages/cli/foundation/discovery/component-replacement.mjs @@ -161,6 +161,24 @@ export async function resolveComponentReplacements( typeof name === 'string' ? byTarget.get(name) : undefined, }; + // A first read under file names alone settles the common case, no + // declaration anywhere, without reading the names docs document. + let declares = false; + for (const integration of loadedIntegrations) { + if (declares || !integration?.components || integration.__loadError) + continue; + try { + const {components} = await discoverValidIntegrationComponents( + integration, + {documentedNames: false}, + ); + declares = components.some(record => record.replaces !== undefined); + } catch { + // Read again below, where a root that cannot be read is skipped. + } + } + if (!declares) return result; + /** @type {Declaration[]} */ const declarations = []; /** @type {Array<{name: string, package: string}>} */ diff --git a/packages/cli/foundation/discovery/component-replacement.test.mjs b/packages/cli/foundation/discovery/component-replacement.test.mjs index 269bed5811a6a..4600459d25b52 100644 --- a/packages/cli/foundation/discovery/component-replacement.test.mjs +++ b/packages/cli/foundation/discovery/component-replacement.test.mjs @@ -259,6 +259,26 @@ describe('resolveComponentReplacements', () => { ]); }); + it('applies a replacement under the export a doc names when its file is named otherwise', async () => { + // Nav.doc.mjs documents AcmeSideNav, the export of a Nav.tsx that exports + // nothing named Nav. + const acme = integrationWithDocSource( + '@acme/nav', + 'Nav', + BASE_DOC.replace("name: 'AcmeNav'", "name: 'AcmeSideNav'"), + ); + fs.writeFileSync( + path.join(acme.components, 'Nav.tsx'), + 'export function AcmeSideNav() { return null; }\n', + ); + const result = await resolveComponentReplacements(CORE_DIR, [acme]); + expect(result.findings).toEqual([]); + expect(result.forTarget('SideNav')).toMatchObject({ + name: 'AcmeSideNav', + package: '@acme/nav', + }); + }); + it('lets a replacement keep the Core name it replaces', async () => { const acme = integration( '@acme/nav', diff --git a/packages/cli/foundation/discovery/module-exports.mjs b/packages/cli/foundation/discovery/module-exports.mjs index 40cae65fdfd03..4c96ffa8f7531 100644 --- a/packages/cli/foundation/discovery/module-exports.mjs +++ b/packages/cli/foundation/discovery/module-exports.mjs @@ -6,14 +6,16 @@ * Proves a runtime export without executing package code. Local ESM re-exports * are followed recursively; unresolved or unparsable modules fail closed. * - * @input A module path and runtime export name - * @output Whether that export can be proved statically + * @input A module path and runtime export name, or a module path and the + * exports to try + * @output Whether that export can be proved statically and the file that + * declares it, or the exported object literal as plain data * @position packages/cli/foundation/discovery — shared package inspection */ import * as fs from 'node:fs'; import * as path from 'node:path'; -import jscodeshift from 'jscodeshift'; +import {parse} from '@babel/parser'; const MODULE_EXTENSIONS = [ '.mjs', @@ -25,7 +27,141 @@ const MODULE_EXTENSIONS = [ '.tsx', '.jsx', ]; -const parser = jscodeshift.withParser('tsx'); +// The options jscodeshift's tsx parser passes to Babel, so the same sources +// parse, without tokens or the recast layer: these reads only look at the tree. +/** @type {import('@babel/parser').ParserOptions} */ +const PARSE_OPTIONS = /** @type {any} */ ({ + sourceType: 'module', + allowImportExportEverywhere: true, + allowReturnOutsideFunction: true, + plugins: [ + 'jsx', + 'asyncGenerators', + 'decoratorAutoAccessors', + 'bigInt', + 'classPrivateMethods', + 'classPrivateProperties', + 'classProperties', + 'decorators-legacy', + 'doExpressions', + 'dynamicImport', + 'exportDefaultFrom', + 'exportExtensions', + 'exportNamespaceFrom', + 'functionBind', + 'functionSent', + 'importAttributes', + 'importMeta', + 'nullishCoalescingOperator', + 'numericSeparator', + 'objectRestSpread', + 'optionalCatchBinding', + 'optionalChaining', + ['pipelineOperator', {proposal: 'minimal'}], + 'throwExpressions', + 'typescript', + ], +}); + +/** + * Modules parsed in this process, by path, reused while the file is unchanged: + * one command can ask about the same module more than once. + * @type {Map} + */ +const parsedModules = new Map(); + +/** + * Module texts read in this process, by path, reused while the file is + * unchanged. + * @type {Map} + */ +const moduleTexts = new Map(); + +/** + * A file's identity for these caches: size and mtime, plus ctime and inode, + * which change when a file is rewritten even if npm stamps the same mtime on it. + * @param {string} file + */ +function fileStamp(file) { + const {size, mtimeMs, ctimeMs, ino} = fs.statSync(file); + return `${size}:${mtimeMs}:${ctimeMs}:${ino}`; +} + +/** @param {string} file */ +function moduleText(file) { + const stamp = fileStamp(file); + const cached = moduleTexts.get(file); + if (cached?.stamp === stamp) return cached.text; + const text = fs.readFileSync(file, 'utf-8'); + moduleTexts.set(file, {stamp, text}); + return text; +} + +/** @param {string} file */ +function parseModule(file) { + const stamp = fileStamp(file); + const cached = parsedModules.get(file); + if (cached?.stamp === stamp) return cached.ast; + const ast = parse(moduleText(file), PARSE_OPTIONS); + parsedModules.set(file, {stamp, ast}); + return ast; +} + +// `export *`: the one way a module exports a name it never writes. +const EXPORT_ALL_RE = /\bexport\s*\*/; + +/** + * Whether `text` has `word` with no identifier character on either side. + * @param {string} text + * @param {string} word + */ +function writesWord(text, word) { + for ( + let at = text.indexOf(word); + at !== -1; + at = text.indexOf(word, at + 1) + ) { + const before = text[at - 1] ?? ''; + const after = text[at + word.length] ?? ''; + if (!/[\w$]/.test(before) && !/[\w$]/.test(after)) return true; + } + return false; +} + +const NOT_CHILDREN = new Set([ + 'loc', + 'start', + 'end', + 'extra', + 'comments', + 'leadingComments', + 'trailingComments', + 'innerComments', +]); + +/** + * Every node of `type` in the tree, at any depth, in source order. + * @param {any} node + * @param {string} type + * @param {any[]} [found] + * @returns {any[]} + */ +function nodesOfType(node, type, found = []) { + if (Array.isArray(node)) { + for (const child of node) nodesOfType(child, type, found); + return found; + } + if (node == null || typeof node !== 'object') return found; + if (node.type === type) found.push(node); + for (const key in node) { + if (NOT_CHILDREN.has(key)) continue; + const child = node[key]; + if (child != null && typeof child === 'object') { + nodesOfType(child, type, found); + } + } + return found; +} /** @param {unknown} specifier @param {string} fromFile */ function resolveLocalModule(specifier, fromFile) { @@ -76,32 +212,56 @@ function runtimeDefault(declaration) { * @param {string} file * @param {string} exportName * @param {Set} [seen] + * @returns {boolean} */ export function moduleExportsName(file, exportName, seen = new Set()) { + return moduleExportSource(file, exportName, seen) != null; +} + +/** + * The file that declares a module's runtime export, without executing it (the + * module, or where a local re-export leads); null when it cannot be proved. + * @param {string} file + * @param {string} exportName + * @param {Set} [seen] + * @returns {string | null} + */ +export function moduleExportSource(file, exportName, seen = new Set()) { const identity = `${file}\0${exportName}`; - if (seen.has(identity)) return false; + if (seen.has(identity)) return null; seen.add(identity); + let text; try { - if (!fs.statSync(file).isFile()) return false; + if (!fs.statSync(file).isFile()) return null; + text = moduleText(file); } catch { - return false; + return null; + } + // A module whose text writes neither the export's name as a word nor + // `export *` cannot export it, so it is not parsed. Any `export *` text, even + // in a comment, leaves the answer to the parsed tree below. + if ( + exportName !== 'default' && + !writesWord(text, exportName) && + !EXPORT_ALL_RE.test(text) + ) { + return null; } - let root; + let ast; try { - root = parser(fs.readFileSync(file, 'utf-8')); + ast = parseModule(file); } catch { - return false; + return null; } - let found = false; - root - .find(parser.ExportNamedDeclaration) - .forEach((/** @type {any} */ exportPath) => { - if (found || exportPath.node.exportKind === 'type') return; - const node = exportPath.node; + /** @type {string | null} */ + let found = null; + nodesOfType(ast.program, 'ExportNamedDeclaration').forEach( + (/** @type {any} */ node) => { + if (found || node.exportKind === 'type') return; if (declarationExports(node.declaration, exportName)) { - found = true; + found = file; return; } for (const specifier of node.specifiers ?? []) { @@ -114,33 +274,185 @@ export function moduleExportsName(file, exportName, seen = new Set()) { const exported = specifier.exported?.name ?? specifier.exported?.value; if (exported !== exportName) continue; if (!node.source) { - found = true; + found = file; return; } const target = resolveLocalModule(node.source.value, file); const imported = specifier.local?.name ?? specifier.local?.value ?? exportName; - if (target && moduleExportsName(target, imported, seen)) { - found = true; + const declared = target + ? moduleExportSource(target, imported, seen) + : null; + if (declared) { + found = declared; return; } } - }); - if (found) return true; + }, + ); + if (found) return found; if (exportName === 'default') { - const defaults = root.find(parser.ExportDefaultDeclaration).nodes(); + const defaults = nodesOfType(ast.program, 'ExportDefaultDeclaration'); if (defaults.some((/** @type {any} */ node) => runtimeDefault(node.declaration))) { - return true; + return file; } } - root - .find(parser.ExportAllDeclaration) - .forEach((/** @type {any} */ exportPath) => { - if (found || exportPath.node.exportKind === 'type') return; - const target = resolveLocalModule(exportPath.node.source?.value, file); - if (target && moduleExportsName(target, exportName, seen)) found = true; - }); + nodesOfType(ast.program, 'ExportAllDeclaration').forEach( + (/** @type {any} */ node) => { + if (found || node.exportKind === 'type') return; + const target = resolveLocalModule(node.source?.value, file); + if (target) found = moduleExportSource(target, exportName, seen); + }, + ); return found; } + +const TS_WRAPPERS = new Set([ + 'TSSatisfiesExpression', + 'TSAsExpression', + 'TSNonNullExpression', + 'ParenthesizedExpression', +]); + +/** @param {any} node */ +function withoutTypeWrappers(node) { + let current = node; + while (current && TS_WRAPPERS.has(current.type)) current = current.expression; + return current; +} + +/** + * The object a module exports, as plain data, read without running it: the + * first of `exportNames` it exports, an object literal in place or a top-level + * binding (under `satisfies` or `as` too). Only literal values are kept; + * anything computed, or overridable by a later spread, is left out. + * + * @param {string} file + * @param {readonly string[]} exportNames `default` or named exports, in order + * @returns {Record | null} + */ +export function staticExportObject(file, exportNames) { + /** @type {any[]} */ + let body; + try { + body = parseModule(file).program.body; + } catch { + return null; + } + + /** @type {Map} top-level bindings to their initializers */ + const bindings = new Map(); + for (const statement of body) { + const declaration = + statement.type === 'ExportNamedDeclaration' + ? statement.declaration + : statement; + if (declaration?.type !== 'VariableDeclaration') continue; + for (const declarator of declaration.declarations) { + if (declarator.id?.type === 'Identifier' && declarator.init) { + bindings.set(declarator.id.name, declarator.init); + } + } + } + + /** @param {string} name @returns {any} undefined when not exported */ + const exported = name => { + for (const statement of body) { + if (name === 'default' && statement.type === 'ExportDefaultDeclaration') { + return statement.declaration; + } + if ( + statement.type !== 'ExportNamedDeclaration' || + statement.exportKind === 'type' + ) { + continue; + } + if (statement.declaration?.type === 'VariableDeclaration') { + const declarator = statement.declaration.declarations.find( + (/** @type {any} */ item) => + item.id?.type === 'Identifier' && item.id.name === name, + ); + if (declarator) return declarator.init ?? null; + } + if (statement.source) continue; + for (const specifier of statement.specifiers ?? []) { + const as = specifier.exported?.name ?? specifier.exported?.value; + if (specifier.type === 'ExportSpecifier' && as === name) { + return {type: 'Identifier', name: specifier.local?.name}; + } + } + } + return undefined; + }; + + /** + * @param {any} node + * @param {Set} following bindings being followed, against cycles + * @returns {unknown} undefined when the value is not a literal + */ + const literal = (node, following) => { + const value = withoutTypeWrappers(node); + switch (value?.type) { + case 'StringLiteral': + case 'NumericLiteral': + case 'BooleanLiteral': + return value.value; + case 'NullLiteral': + return null; + case 'TemplateLiteral': + return value.expressions.length === 0 + ? value.quasis[0]?.value.cooked + : undefined; + case 'Identifier': { + if (!bindings.has(value.name) || following.has(value.name)) { + return undefined; + } + const next = new Set(following).add(value.name); + return literal(bindings.get(value.name), next); + } + case 'ArrayExpression': + return value.elements + .map((/** @type {any} */ element) => + element == null || element.type === 'SpreadElement' + ? undefined + : literal(element, following), + ) + .filter((/** @type {unknown} */ item) => item !== undefined); + case 'ObjectExpression': { + /** @type {Record} */ + let object = {}; + for (const property of value.properties) { + if (property.type === 'SpreadElement') { + object = {}; + continue; + } + const key = property.computed + ? undefined + : (property.key?.name ?? property.key?.value); + if (typeof key !== 'string') continue; + const item = + property.type === 'ObjectProperty' + ? literal(property.value, following) + : undefined; + if (item === undefined) delete object[key]; + else object[key] = item; + } + return object; + } + default: + return undefined; + } + }; + + for (const exportName of exportNames) { + const node = exported(exportName); + if (node === undefined) continue; + const value = literal(node, new Set()); + return value != null && typeof value === 'object' && !Array.isArray(value) + ? /** @type {Record} */ (value) + : null; + } + return null; +} diff --git a/packages/cli/foundation/discovery/module-exports.test.mjs b/packages/cli/foundation/discovery/module-exports.test.mjs new file mode 100644 index 0000000000000..3da8ee17c4081 --- /dev/null +++ b/packages/cli/foundation/discovery/module-exports.test.mjs @@ -0,0 +1,172 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Static reads of a module's exports, without running it: the file that + * declares an export, and an exported object literal as plain data. + */ + +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import {moduleExportSource, staticExportObject} from './module-exports.mjs'; + +/** @type {string} */ +let dir; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-export-field-')); +}); + +afterEach(() => { + fs.rmSync(dir, {recursive: true, force: true}); +}); + +/** + * @param {string} file + * @param {string} source + * @returns {string} + */ +function write(file, source) { + const filePath = path.join(dir, file); + fs.writeFileSync(filePath, source); + return filePath; +} + +const DOC_EXPORTS = ['default', 'docs']; + +describe('staticExportObject', () => { + it('reads a default-exported object literal as plain data', () => { + const file = write( + 'A.doc.mjs', + "export default {type: 'component', name: 'AcmeCarousel', order: 2, hidden: false, extra: null};\n", + ); + expect(staticExportObject(file, DOC_EXPORTS)).toEqual({ + type: 'component', + name: 'AcmeCarousel', + order: 2, + hidden: false, + extra: null, + }); + }); + + it('reads a named export, an export list, and the first export it looks for', () => { + const named = write( + 'B.doc.mjs', + "/** @type {object} */\nexport const docs = {\n name: 'AcmeCarousel',\n};\n", + ); + const both = write( + 'C.doc.mjs', + "export const docs = {name: 'Legacy'};\nexport default {name: 'Stamped'};\n", + ); + const list = write( + 'E.doc.mjs', + "const local = {name: 'AcmeCarousel'};\nexport {local as docs};\n", + ); + expect(staticExportObject(named, DOC_EXPORTS)?.name).toBe('AcmeCarousel'); + expect(staticExportObject(both, DOC_EXPORTS)?.name).toBe('Stamped'); + expect(staticExportObject(list, DOC_EXPORTS)?.name).toBe('AcmeCarousel'); + }); + + it('follows top-level bindings through TypeScript wrappers, at any depth', () => { + const file = write( + 'D.doc.ts', + [ + "const slide = {name: 'AcmeSlide', description: 'One slide.'} as const;", + "const doc = {name: 'AcmeCarousel', components: [slide, {name: 'AcmeDots', description: 'Dots.'}]} satisfies {name: string};", + 'export default doc;', + '', + ].join('\n'), + ); + expect(staticExportObject(file, DOC_EXPORTS)).toEqual({ + name: 'AcmeCarousel', + components: [ + {name: 'AcmeSlide', description: 'One slide.'}, + {name: 'AcmeDots', description: 'Dots.'}, + ], + }); + }); + + it('leaves out what it cannot read without running the module', () => { + const file = write( + 'F.doc.mjs', + [ + "const prefix = 'Acme';", + 'const shared = {group: "Media"};', + 'export default {', + ' // A computed string, a call and a spread: none is read.', + ' name: `${prefix}Carousel`,', + ' usage: makeUsage(),', + " group: 'Overridden',", + ' ...shared,', + " category: 'Media',", + " components: [...more, {name: 'AcmeSlide'}],", + '};', + '', + ].join('\n'), + ); + expect(staticExportObject(file, DOC_EXPORTS)).toEqual({ + category: 'Media', + components: [{name: 'AcmeSlide'}], + }); + }); + + it('answers null when there is no exported object', () => { + const files = [ + write( + 'G.doc.mjs', + "export const docs = {name: 'Legacy'};\nexport default makeDoc();\n", + ), + write('H.doc.mjs', 'export default {name: '), + write('I.doc.mjs', "export const other = {name: 'AcmeCarousel'};\n"), + write('J.doc.mjs', "export default ['AcmeCarousel'];\n"), + ]; + for (const file of files) { + expect(staticExportObject(file, DOC_EXPORTS)).toBeNull(); + } + }); +}); + +describe('moduleExportSource', () => { + it('names the file that declares an export, following local re-exports', () => { + const impl = write( + 'impl.tsx', + 'export function AcmeSlide() { return null; }\n', + ); + const barrel = write( + 'index.ts', + "export {AcmeSlide} from './impl';\nexport * from './dots';\nexport function AcmeCarousel() { return null; }\n", + ); + const dots = write('dots.tsx', 'export const AcmeDots = () => null;\n'); + expect(moduleExportSource(barrel, 'AcmeSlide')).toBe(impl); + expect(moduleExportSource(barrel, 'AcmeDots')).toBe(dots); + expect(moduleExportSource(barrel, 'AcmeCarousel')).toBe(barrel); + expect(moduleExportSource(barrel, 'AcmeMissing')).toBeNull(); + }); + + it('does not follow a re-export written in a comment', () => { + write('legacy.tsx', 'export function AcmeTab() { return null; }\n'); + const barrel = write( + 'tabs.ts', + "// export * from './legacy';\nexport function AcmeTabs() { return null; }\n", + ); + expect(moduleExportSource(barrel, 'AcmeTab')).toBeNull(); + }); + + it('does not count a type-only export', () => { + const file = write('types.ts', 'export type AcmeShape = {id: string};\n'); + expect(moduleExportSource(file, 'AcmeShape')).toBeNull(); + }); + + it('sees a file npm replaced with one of the same size and mtime', () => { + // npm stamps every installed file with one mtime and writes new files. + const npmTime = new Date('1985-10-26T08:15:00Z'); + const file = write('Panel.tsx', 'export function AcmePanel() {}\n'); + fs.utimesSync(file, npmTime, npmTime); + expect(moduleExportSource(file, 'AcmePanel')).toBe(file); + const next = write('Panel.next.tsx', 'export function AcmePanol() {}\n'); + fs.utimesSync(next, npmTime, npmTime); + fs.renameSync(next, file); + expect(moduleExportSource(file, 'AcmePanel')).toBeNull(); + }); +});