diff --git a/.changeset/cli-fix-integration-import-specifier.md b/.changeset/cli-fix-integration-import-specifier.md new file mode 100644 index 000000000000..8e8efb28731d --- /dev/null +++ b/.changeset/cli-fix-integration-import-specifier.md @@ -0,0 +1,9 @@ +--- +'@astryxdesign/cli': patch +--- + +[fix] `component` built the import specifier for an integration component by joining the package name and the component name, which assumes every component is exported from a subpath named after itself. Components are commonly grouped behind a single entry point named after the concept, so the suggested import pointed at a subpath the package does not export and did not resolve. + +The specifier is now resolved against the owning package's `exports` map, keyed on the directory the component's doc file sits in, and falls back to the package root when that directory is not an exported subpath. A specifier a doc file states for itself is also no longer overwritten. + +@rubyycheung diff --git a/packages/cli/api/component/_adapter.mjs b/packages/cli/api/component/_adapter.mjs index 971892936a09..06b3877e5cae 100644 --- a/packages/cli/api/component/_adapter.mjs +++ b/packages/cli/api/component/_adapter.mjs @@ -18,6 +18,8 @@ * deduped, so each leaf stays a thin projection. */ +import * as fs from 'node:fs'; +import * as path from 'node:path'; import {ERROR_CODES} from '../../foundation/response/error-codes.mjs'; import {findCoreDir, discoverExternalPackages} from '../../foundation/fs/paths.mjs'; import { @@ -48,6 +50,7 @@ export {CORE_PACKAGE}; * @property {any[]} [components] * @property {{description?: string}} [usage] * @property {any} [theming] + * @property {string} [import] set when the doc states its own import specifier */ /** @@ -68,6 +71,13 @@ export {CORE_PACKAGE}; * @property {import('../../foundation/integrations/integrations.mjs').LoadedIntegration|null} integration */ +/** + * What ownership shaping needs from an owner. The core and legacy-external + * paths synthesize a bare `{package, sourcePath}` rather than resolving a full + * {@link ComponentOwner}, so everything past those two is optional here. + * @typedef {Partial & {package: string, sourcePath: string|null}} OwnershipSubject + */ + /** * A back-compat external package discovered via `pkg.astryx.docs`. * @typedef {{name: string, category: string, docsDir: string}} ExternalPackageRef @@ -357,7 +367,7 @@ export function extractProps(docs) { * swizzleable source file exists for the owner). Existing doc fields (name, * usage, props, …) are preserved. * @param {LoadedComponentDoc} docs - * @param {{package: string, sourcePath: string|null}} owner + * @param {OwnershipSubject} owner * @param {string} componentName * @param {string} coreDir * @returns {import('./component.type.mjs').ComponentDetailResponse['data']} @@ -366,15 +376,53 @@ export function withOwnership(docs, owner, componentName, coreDir) { const importSpec = owner.package === CORE_PACKAGE ? resolveImportPath(coreDir, componentName) - : `${owner.package}/${componentName}`; + : resolveIntegrationImportPath(owner, componentName); return /** @type {any} */ ({ ...docs, package: owner.package, - import: importSpec, + // A doc file may state its own specifier, e.g. when one entry point exports + // several components. Only fall back to a resolved one when it does not. + import: docs.import ?? importSpec, sourceAvailable: owner.sourcePath != null, }); } +/** + * Resolve the specifier an integration component is imported from, against the + * owning package's `exports` map. + * + * A component lives in a directory that need not share its name — several + * components can be exported from one entry point — so the specifier has to + * come from the directory the doc file sits in, checked against `exports`, + * rather than from the component name. Falls back to the package root when the + * directory is not an exported subpath, matching what a consumer would have to + * write by hand. + * + * @param {OwnershipSubject} owner + * @param {string} componentName + * @returns {string} + */ +function resolveIntegrationImportPath(owner, componentName) { + const packageDir = owner.integration?.__packageDir; + const directory = owner.docPath + ? path.basename(path.dirname(owner.docPath)) + : componentName; + if (!packageDir) { + return owner.package; + } + try { + const manifest = JSON.parse( + fs.readFileSync(path.join(packageDir, 'package.json'), 'utf-8'), + ); + if (manifest.exports?.[`./${directory}`]) { + return `${owner.package}/${directory}`; + } + } catch { + // An unreadable or malformed manifest is not worth failing a lookup over. + } + return owner.package; +} + /** * When the caller asked for "Code" but the resolved doc is for "CodeBlock" * (parent), scope the response to just the matching sub-component. Returns the diff --git a/packages/cli/clients/cli/commands/component-ownership.test.mjs b/packages/cli/clients/cli/commands/component-ownership.test.mjs index a22b07e02a9c..a9887902ea28 100644 --- a/packages/cli/clients/cli/commands/component-ownership.test.mjs +++ b/packages/cli/clients/cli/commands/component-ownership.test.mjs @@ -47,7 +47,12 @@ const INTEGRATION_ISSUES = 'https://example.com/meta/issues'; * Returns the absolute `components` dir so the Project.load mock can hand back a * resolved integration entry. */ -function createFixture({withSource = true, extraComponent = null} = {}) { +function createFixture({ + withSource = true, + extraComponent = null, + packageExports = null, + entryPoint = null, +} = {}) { const realCoreDir = path.resolve(import.meta.dirname, '..', '..', '..', '..', 'core'); const coreDir = path.join(tmpDir, 'packages', 'core'); fs.mkdirSync(path.dirname(coreDir), {recursive: true}); @@ -58,7 +63,11 @@ function createFixture({withSource = true, extraComponent = null} = {}) { fs.mkdirSync(compDir, {recursive: true}); fs.writeFileSync( path.join(intDir, 'package.json'), - JSON.stringify({name: INTEGRATION_NAME, version: '1.2.3'}), + JSON.stringify({ + name: INTEGRATION_NAME, + version: '1.2.3', + ...(packageExports ? {exports: packageExports} : {}), + }), ); fs.writeFileSync( path.join(compDir, 'MetaAppShell.doc.mjs'), @@ -81,6 +90,20 @@ function createFixture({withSource = true, extraComponent = null} = {}) { ); } + // A component whose directory is an entry point exporting several + // components, so the directory name and the component name differ. + if (entryPoint) { + const entryDir = path.join(compDir, entryPoint.directory); + fs.mkdirSync(entryDir, {recursive: true}); + const ownSpecifier = entryPoint.importSpec + ? `\n import: '${entryPoint.importSpec}',` + : ''; + fs.writeFileSync( + path.join(entryDir, `${entryPoint.component}.doc.mjs`), + `export const docs = {\n name: '${entryPoint.component}',${ownSpecifier}\n usage: { description: '${entryPoint.component} from an entry point.' },\n};\n`, + ); + } + const integration = { name: INTEGRATION_NAME, version: '1.2.3', @@ -88,6 +111,7 @@ function createFixture({withSource = true, extraComponent = null} = {}) { templates: undefined, codemods: undefined, issuesUrl: INTEGRATION_ISSUES, + __packageDir: intDir, }; projectLoadMock.mockResolvedValue({ integrations: [INTEGRATION_NAME], @@ -162,7 +186,9 @@ describe('component() — integration ownership via config', () => { expect(result.data.name).toBe('MetaAppShell'); expect(result.data.package).toBe(INTEGRATION_NAME); expect(result.data.sourceAvailable).toBe(true); - expect(result.data.import).toBe(`${INTEGRATION_NAME}/MetaAppShell`); + // This fixture declares no `exports`, so there is no subpath to import + // from and the specifier is the package root. + expect(result.data.import).toBe(INTEGRATION_NAME); }); it('--package resolves the integration component', async () => { @@ -217,6 +243,40 @@ describe('component() — integration ownership via config', () => { expect(result.data.package).toBe(INTEGRATION_NAME); }); + it('resolves the import specifier against the package exports map', async () => { + // The doc sits in a `Toolbar` directory but the component is + // `ToolbarSearch`, so a specifier built from the component name would + // point at a subpath the package does not export. + createFixture({ + packageExports: {'.': './index.js', './Toolbar': './components/Toolbar/index.js'}, + entryPoint: {directory: 'Toolbar', component: 'ToolbarSearch'}, + }); + const result = await component('ToolbarSearch', {cwd: tmpDir}); + expect(result.data.import).toBe(`${INTEGRATION_NAME}/Toolbar`); + }); + + it('falls back to the package root when the directory is not an exported subpath', async () => { + createFixture({ + packageExports: {'.': './index.js'}, + entryPoint: {directory: 'Toolbar', component: 'ToolbarSearch'}, + }); + const result = await component('ToolbarSearch', {cwd: tmpDir}); + expect(result.data.import).toBe(INTEGRATION_NAME); + }); + + it('keeps a specifier the doc file states for itself', async () => { + createFixture({ + packageExports: {'.': './index.js', './Toolbar': './components/Toolbar/index.js'}, + entryPoint: { + directory: 'Toolbar', + component: 'ToolbarSearch', + importSpec: `${INTEGRATION_NAME}/Toolbar/Search`, + }, + }); + const result = await component('ToolbarSearch', {cwd: tmpDir}); + expect(result.data.import).toBe(`${INTEGRATION_NAME}/Toolbar/Search`); + }); + it('JSON list includes integration components as {name, package} objects', async () => { createFixture(); const result = await component(undefined, {cwd: tmpDir, list: true});