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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/cli-fix-integration-import-specifier.md
Original file line number Diff line number Diff line change
@@ -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
54 changes: 51 additions & 3 deletions packages/cli/api/component/_adapter.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
*/

/**
Expand All @@ -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<ComponentOwner> & {package: string, sourcePath: string|null}} OwnershipSubject
*/

/**
* A back-compat external package discovered via `pkg.astryx.docs`.
* @typedef {{name: string, category: string, docsDir: string}} ExternalPackageRef
Expand Down Expand Up @@ -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']}
Expand All @@ -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
Expand Down
66 changes: 63 additions & 3 deletions packages/cli/clients/cli/commands/component-ownership.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand All @@ -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'),
Expand All @@ -81,13 +90,28 @@ 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',
components: compDir,
templates: undefined,
codemods: undefined,
issuesUrl: INTEGRATION_ISSUES,
__packageDir: intDir,
};
projectLoadMock.mockResolvedValue({
integrations: [INTEGRATION_NAME],
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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});
Expand Down
Loading