Repository navigation
feat(cli): integration family members are components under their own names - #7674
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Visual Regression40 of 478 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 20 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval is on hold until a maintainer settles the spec for this capability. No approved record on main makes family members components under their own names, and the member rule exists only in draft spec PR #7658 (spec:AST-039 FR4 and an addition to architecture:cli-surface INV24). A maintainer needs to approve that rule, identify another approved record that covers it, or decline it. Two fixes are needed either way:
- When a family doc's own
nameis one of its members,component <that member>and--propsreturn the whole family doc (every member's props and the family'sreplaces) instead of that member's own entry. - A family doc that writes
componentsas a shorthand property (export default {..., components}) lists no members, because the quickcomponents:text check requires a colon.
[Automated review]
| ); | ||
| } | ||
| const sub = scopeSubComponent(docs, dirName, coreDir); | ||
| if (!sub) return docs; |
There was a problem hiding this comment.
When the family doc's own name is this member (for example Chip.doc.mjs with name: 'AcmeChip' beside a Chip.tsx that also exports Chip), scopeSubComponent declines to scope, and this line returns the whole family doc. component AcmeChip then shows the family's replaces, and --props returns every member's props. Building the scoped doc from integrationMemberEntry(docs, record) here would keep a member scoped to its own entry.
| const NAME_FIELD_RE = /(?:^|\n) {0,2}['"]?name['"]?\s*:\s*['"]([^'"\n]+)['"]/g; | ||
| // A `components:` field anywhere in the file: the quick check for a family doc, | ||
| // before anything parses it. A false match only costs the parse. | ||
| const MEMBERS_FIELD_RE = /\bcomponents['"]?\s*:/; |
There was a problem hiding this comment.
This check misses a shorthand property (export default {..., components}), so such a family doc lists no members even though staticExportObject resolves the binding. Matching \bcomponents\b here would cover it.
38fcb61 to
f84ce11
Compare
|
Addressed the automated review in this push:
|
154b3a1 to
db4b024
Compare
f84ce11 to
96f5bed
Compare
There was a problem hiding this comment.
Approval is on hold until a maintainer settles the spec for this capability: no approved record on main makes family members components under their own names, and the member rule exists only in draft spec PR #7658 (spec:AST-039 FR4 and an addition to architecture:cli-surface INV24). A maintainer needs to approve that rule on main, identify another approved record that covers it, or decline it. The two fixes from the earlier review hold up on this head, but one small fix is still needed either way:
- When a family doc keeps its file name while its
nameis one of its members (such asChip.doc.mjsnamingAcmeChip), a sibling member'sparentDocnamesAcmeChip, which opens that member instead of the family doc.
[Automated review]
| parentDoc: | ||
| docs.name === dirName && record.docPath | ||
| ? docStem(record.docPath) | ||
| : docs.name, |
There was a problem hiding this comment.
When the family doc keeps its file name while its name is one of its members (for example Chip.doc.mjs with name: 'AcmeChip' beside a Chip.tsx that exports Chip, AcmeChip and AcmeChipIcon), this gives AcmeChipIcon a parentDoc of AcmeChip. component AcmeChip then opens that member rather than the family doc, while AcmeChip itself gets Chip. Naming the family by the name discovery records the doc under (its stem, or the export its name documents when it is recorded under that export) would give every member the same parent, one that opens the family doc.
db4b024 to
e75b79f
Compare
96f5bed to
01cc412
Compare
|
Addressed the automated review in 01cc412: every member's |
e75b79f to
3fb88dc
Compare
01cc412 to
eb50a01
Compare
61969eb to
7db5b0f
Compare
05eca4d to
4e51e99
Compare
|
Addressed in 4e51e99. For a family whose import file the pack lifecycle wrote, verify now compares that family's members as local discovery lists them after the pack, instead of excusing packed-only names. A stale build that still exports a member the new build drops no longer fails with |
There was a problem hiding this comment.
Approval is on hold until a maintainer settles the spec for this capability: no approved record on main makes family members components under their own names, and the member rule exists only in draft spec PR #7658 (spec:AST-039 FR4 and an addition to architecture:cli-surface INV24). A maintainer needs to approve that rule on main, identify another approved record that covers it, or decline it. The stale-build integration verify fix holds up, and one small fix is still needed either way:
- For a family whose
importthe pack lifecycle builds, a passingintegration verify --jsoncompares the members listed after the pack but still reportscontributions.localfrom before it, so the local and packed component lists disagree (for example["AcmeTabs"]against["AcmeTab", "AcmeTabs"]from a clean checkout); please report the compared list and assert that the two inventories match in the lifecycle pack-check case.
[Automated review]
| } | ||
| issues.push( | ||
| ...compareIdentities( | ||
| {...localIdentities, components: localComponents}, |
There was a problem hiding this comment.
This compares the members local discovery lists after the pack, but the result still returns contributions.local from before it (localIdentities, line 1500). From a clean checkout with a prepack-built dist, a passing run reports local.components as ["AcmeTabs"] against packed.components as ["AcmeTab", "AcmeTabs"], and when the tarball genuinely loses AcmeTab, the identity_not_packed issue names a member the local list omits. Returning {...localIdentities, components: localComponents} as contributions.local, and asserting that contributions.packed equals contributions.local in the lifecycle pack-check case, would keep the report consistent with what verify compared.
4e51e99 to
bdb8bff
Compare
|
Addressed in bdb8bff: the receipt now reports the local inventory that verify compared, so for a family whose import the pack lifecycle builds, |
There was a problem hiding this comment.
Approval is on hold until a maintainer settles the spec for this capability: no approved record on main makes family members components under their own names, and the member rule exists only in draft spec PR #7658 (spec:AST-039 FR4 and an addition to architecture:cli-surface INV24). A maintainer needs to approve that rule on main, identify another approved record that covers it, or decline it. The integration verify receipt fix holds up, and one small fix is still needed either way:
- For a family doc at an integration's package root (
components: '.'), a member that only the built bundle declares is listed withdist/index.mjsas its source, so--sourceprints the bundle andswizzlecopiesdist/, although the new guide says such a member is not listed.
[Automated review]
| const exported = moduleExportSource(module, name); | ||
| if (exported == null) return []; | ||
| const sourcePath = [moduleExportSource(familySource, name), exported].find( | ||
| file => file != null && isWithin(folder, file), |
There was a problem hiding this comment.
When the family doc sits at the package root (an integration with components: '.'), folder is the package directory, so a member only the bundle declares gets dist/index.mjs as its source: it is listed, component <Member> --source prints the bundle, and swizzle copies index.mjs and index.mjs.map. The new guide says such a member is not listed. Keeping a bundle out of this search when the family folder contains it, with a discovery case for a root-level family doc, would match the guide; otherwise the guide should say that a bundle inside the family folder counts.
bdb8bff to
216b867
Compare
|
Addressed in 216b867: a member's source must now be a |
There was a problem hiding this comment.
Approval is on hold until a maintainer settles the spec for this capability: no approved record on main says that each full entry a family doc lists in components is a component under its own name, and that rule exists only in draft spec PR #7658 (spec:AST-039 FR4 and an addition to architecture:cli-surface INV24). A maintainer needs to approve that rule on main, identify another approved record that covers it, or decline it, and the decision should also cover how integration verify compares members of a family whose import the pack lifecycle builds. The bundle-source fix in this push holds up, and this review found nothing else to change.
[Automated review]
…names Each full entry a family doc lists in `components` is a component under its own name when the module the doc's import names exports it: the doc's `import`, else the subpath its directory publishes, read through the package's `exports`, or its `main` when it has none. An import the package cannot resolve lists no members. The member's source is the file that declares it, reached through the family's own file or the imported module, and it must sit in the family doc's folder; a bundle or a package's root file is no member's source, and a family doc without its own source lists none. Static PascalCase names, not hidden, not the doc's stem or the export it is recorded under, and not a name already taken (a file name, hidden docs included, or a Core name the package replaces). A `components:` value held by a binding, and an entry with computed fields, count. Members follow the owner rule like export names. Component detail and --props scope to the member's entry with the family's import, and fail rather than answer with the family when the doc a long-lived process loaded has no entry for it. Search indexes a member from its own entry, deciding member or own from the files, not from whether its doc loads. A member never carries the family's replaces; a family doc that fails to load is reported once (verify and the contribution inventory load it once). `integration add component` refuses a name discovery records and plans one it does not.
…and family members Add a component gains a section for documenting a component the package already has: name the doc after its source file, such as acme-gallery.doc.mjs beside acme-gallery.tsx, and set name to the export apps import. The single component, import, checks, troubleshooting and versioning guides, and the ComponentDoc components field, now say what discovery and integration verify do with a doc that names another export of its file and with family members, what an app on an older CLI sees, and how a name Core or another owner keeps is reached with --package.
216b867 to
86179ee
Compare
There was a problem hiding this comment.
Changes requested: the member rule now has current spec support, but discovery silently drops some members that the doc's import exports, which the spec says must be listed. When import names compiled or bundled output (for example a dist/list/index.js that re-exports ./row.js), a member declared in its own .tsx beside the family doc is never found unless the family's own file re-exports it, and a member re-exported from another folder is dropped too. In both cases integration verify still passes, and integration add component offers to create a second doc. Please look for the member's declaring .tsx in the family folder when the import's chain does not reach one, record or report the rest instead of dropping them (or get the spec amended if that condition is intended), add a test with compiled dist output, and align the family guide, the ComponentDoc text and the Changeset.
[Automated review]
| const sourcePath = [moduleExportSource(familySource, name), exported].find( | ||
| file => file != null && file.endsWith('.tsx') && isWithin(folder, file), |
There was a problem hiding this comment.
A member that the import exports is recorded only when one of these two routes reaches a .tsx inside the family folder. When import names compiled or bundled output, for example a dist/list/index.js that re-exports ./row.js, neither route reaches src/list/row.tsx. AcmeListRow is then silently not listed, integration verify passes, and integration add component AcmeListRow plans a second doc. INV24 in docs/architecture/cli-surface.md records every full entry the imported module exports. To match it, look for the .tsx in the family folder that exports the member when the chain reaches none, and report an exported entry that has no such file instead of dropping it. A test with compiled dist output would cover this.
| }, | ||
| { | ||
| type: 'prose', | ||
| text: "A member's source is the `.tsx` file that declares it, reached through the family's own file or the module `import` names, and it must sit in the family doc's folder or below it. A member is not listed when only a bundle or a file outside that folder declares it, when a doc's file or a Core name your package replaces already has its name, or when the family doc has no source file of its own. A member that shares a name with Core opens with `--package`.", |
There was a problem hiding this comment.
AcmeListRow, declared in src/list/row.tsx inside the family folder, is still not listed when import names compiled output and the family's own file does not re-export it. This sentence, and the dist sentence above, therefore promise more than discovery does. Please align them with the discovery fix, or say plainly that with built output the family's own .tsx must declare or re-export each member.
|
Fixed in 2d979e3:
Each new test fails without its fix. |
architecture:cli-surface INV24 records each full entry a family doc lists when the module its import names exports it. A member whose import reaches no .tsx file, as when it names built output, now takes its source from the .tsx file in the family's folder that declares it; a member only a bundle or a file outside that folder declares is listed with no source, and is valid: only a doc's own component needs its same-stem file. A family doc without its own source lists the members its import exports. integration verify compares every import family's members as local discovery lists them after the pack, so an incremental build that leaves the imported file untouched no longer reports the new member as unexpected.
af29e54 to
2d979e3
Compare
There was a problem hiding this comment.
This is ready to merge: family members now resolve under their own names on every CLI surface, as architecture:cli-surface INV24 describes, and the earlier gap where members behind compiled output or from another folder were silently dropped is fixed. Exact lookups that Core, legacy docs packages and file-named docs answer are unchanged, and --package reaches a member that shares their name. No further changes are needed.
[Automated review]
…ion-guides-cleanup ComponentDoc.name takes #7659's sentence about the export apps import in place of the file-name rule. The versioning list keeps this branch's published-version bullets and adds #7674's family-member bullet. hiddenComponents now says what #7674's discovery does in an integration: it hides full entries in the doc's own `components`, and a sub-component with its own doc still uses `hidden`.
User need
A family doc lists several components in
components, and the component-family guide already requires the module named by itsimportto export every full entry. Yet only the doc's own component is reachable:AcmeTabin anAcmeTabsfamily doc is not in the list,component AcmeTabfails withERR_UNKNOWN_COMPONENT, search never shows it, andswizzle AcmeTabcannot copy it.Problem and solution fit
Each full entry a family doc lists in
componentsbecomes a component under its own name when the module the doc's import names exports it. It follows #7659's owner rule: a member holds its name only because a doc records it, so it gives way on an exact unscoped lookup to Core and to any doc named after its file, and--packagestill reaches it.import, else the subpath its directory publishes, read through the package'sexports. A package withoutexportsresolves its bare name throughmain, thenindex. When the package directory is known but the import resolves to no file, the doc lists no members; there is no fallback to the same-stem file..tsxfile in the family doc's folder, or below it, that declares the member: the one the family's own file or the imported module reaches, else the one in that folder that declares the name, as when the import names built output. A bundle (dist/index.mjs) or a file outside the folder is never a member's source; a member only such a file declares is still listed, with no source, so--sourceandswizzleanswerERR_NO_SOURCEfor it. It is valid: only a doc's own component needs its same-stem file.componentsfield written out or as a shorthand, not inhiddenComponents, not the doc's file name or the export it is recorded under, and not taken: every doc's file name (hidden docs included), a Core name the package replaces, or an earlier claim. An entry is full when it has more thanname, including computed fields and spreads, and acomponentsvalue held by a binding is read.component AcmeTaband--propsshow the member's own entry (description, props, examples) with the family's import, the way a Core sub-component's detail does, including a member that shares the family doc's ownname. Every member'sparentDocis the name the family doc opens under (its file name, or the export it is recorded under). Search indexes a member from its own entry. A member never carries the family'sreplaces. A family doc that fails to load is reported once, not once per member, byintegration verifyand the contribution inventory.integration add componentreads the import the way discovery does: it refuses a name discovery records and plans one it does not.Current specification
The current record
architecture:cli-surfaceINV24, with the details inspec:AST-039FR4 (both from #7658, merged as c992783): discovery records each full entry a descriptor lists incomponentsunder its own name when the module the descriptor imports exports it, and a name recorded only this way never takes an unscoped lookup that Core, a legacy docs package, or a component named after its source answers.Public delta and caller burden
astryx component AcmeTabexits 1 withERR_UNKNOWN_COMPONENT. After: it printsAcmeTab's own entry withimport: @acme/widgets/tabs, andcomponent AcmeTab --propsreturns its props.Dialog) or a Core sub-component (ListItem) leaves the bare name to Core on every surface.--package @acme/widgetsreaches the member, and search shows it with that command.integration verifydoes not report it. When the import names output the pack lifecycle builds (such asdist), the members are listed once that output exists, andintegration verifycompares every import family's members as local discovery lists them after the pack, so a clean checkout, a stale build, and an incremental build that leaves the imported file untouched all pass.acme-gallery.doc.mjsbesideacme-gallery.tsx, kebab case included) and setnameto the export. Single component, Resolve the import, Check an integration, Troubleshooting and Versioning, and theComponentDoccomponentsfield, say whatintegration verifychecks about the export (a type-only export does not count), which messages a doc that still names its file produces, how a name Core keeps is reached with--package, and what an app on an older CLI sees.[feat].Design and alternatives
foundation/discovery/component-discovery.mjs:familyMembersreads a family doc's full entries, resolves the module, and keeps each member the module exports, with its source from the family folder (folderSourceForwhen the import's chain reaches no.tsxfile).integrationMemberEntrygives a member's entry, and a doc's own record is decided by its identity, never by a lookup incomponents.api/component/_adapter.mjs:scopeIntegrationMemberscopes detail to the member's entry. When the doc a long-lived process loaded has no entry for a member the files now list, detail fails withERR_UNKNOWN_COMPONENTinstead of answering with the family.api/search/search.mjs: indexes a member from its own entry and decides member or own from the files, so a.doc.tsfamily undernode_moduleskeeps its members.api/integration/pack-check.mjsandfoundation/integrations/contribution-inventory.mjs: load each doc once.integration verifycompares each import family's members as listed after the pack.api/integration/add-contribution.mjs: passes the loaded integration so names follow discovery.Rejected: making
componentsentries name-only pointers that each need their own doc (forces a doc per member, which the family guide exists to avoid); listing members from the same-stem file when the import cannot be resolved (the list would name components apps cannot import).Evidence
integration verifyis packable with all 40 components.exports, a member declared only in a bundle, a withdrawn family, a binding-held list, a.doc.tsfamily, a family doc the loader rejects): every check passes, every surface agrees on each member's owner, and nothing is reported twice.Scope
Testing
component-discovery.export-names.test.mjs: members frommainand none from an unresolvable import; members listed through a shorthandcomponentsproperty; a source in the family folder, never a bundle or the package root, even for a family doc at the package root; a member's source found in the family folder when the import names built output; a member only a bundle, the package root or another folder declares, listed with no source and valid; the members a family without its own source imports; a binding-held list and computed entries; the doc's own name as a member withreplacesdeclared once; hidden-doc and replacement names taken.integration-export-name.test.mjs: the member variant of the Core-name test, search command included; a member that shares its family doc's ownnameopens as that member, with its own props, and a sibling member'sparentDocopens the family; a member named like a Core sub-component stays unknown to swizzle; a stale loaded family doc fails instead of answering with the family; a.doc.tsfamily member under the real CLI.add-contribution.component-names.test.mjs: plans a name the family lists but its import does not export, and refuses a member built output exports.pack-check.test.mjsandcomponent-replacement.test.mjs: members checked against the imported module; members read from output the pack lifecycle builds, from a clean checkout, a stale build missing a member, and a stale build still exporting one, each reporting matching local and packed inventories; members built output re-exports from compiled files, from a clean checkout and from an incremental build that leaves the imported file untouched; a member another folder declares, which has no source, passes; a rejected family doc reported once; a family'sreplacesstays with the family.@astryxdesign/theme-neutral/builterrors.