feat(grid): publish the grid contract at its registry key - #74
Merged
Conversation
Publish the grid capability's structural contract over the mechanism the theming contract established. `plugins/grid/contract.ts` declares the grid vocabulary as types alone and `package.json` publishes it at `./grid` under a `types` condition with no runtime condition beside it, so the capability's value keeps coming from the registry alone. The contract takes `ThemeVariable` from `@fx/tx/theme` rather than from a relative path into `../theme/`. A relative import is the escape the boundary test exists to forbid — it would put two bundled plugins in one runtime module graph — while the published path shares no graph at all, because it is erased. That deletes the theme vocabulary `plugins/grid/theme.ts` carried, leaving it the capability lookup alone, and it is the first cross-contract import the arrangement was built for: the file list the package publishes has to be closed over it, which is what the consumer fixture now proves against the tarball. The registry key becomes that same specifier in the same commit. A key that named a specifier publishing nothing would be a tx violating the rule its own specification states, so `grid` is renamed at the provider, at every consumer lookup, in the demo, and in the plugin guide. This is a breaking change to anything reading the capability and is accepted as one: no alias and no dual registration, because registering under both spellings would put two entries in one snapshot. The plugin's own identity name stays bare — that names the plugin, not the key. `plugins/grid/types.ts` becomes that contract rather than sitting beside it: the two would otherwise be one vocabulary declared twice, which is the duplication publication exists to remove. Claude-Session: https://claude.ai/code/session_0165ZoEriyA7L7iLAG81AkR3
…ontracts The bundled consumers of the theming capability, the demo's catalogue, and the capability tests each carried a structural copy of the shape they read from the registry. A copy compiles whatever the provider does, so a contract that moved was discovered when a command reached for a member that was no longer there rather than when the consumer was built — which is exactly what publishing the contracts exists to stop, and it buys nothing while the copies remain. Each of them now imports the contract published at the key it reads. The dialogs plugin keeps its theme lookup and loses the vocabulary around it, the demo's catalogue keeps only the stream-less printing request the runner needs and takes the rest from `@fx/tx/grid` and `@fx/tx/theme`, and the theming and grid suites read the capability through the published types an external consumer would use. The dialog vocabulary stays restated in the demo: that contract is not published yet. Claude-Session: https://claude.ai/code/session_0165ZoEriyA7L7iLAG81AkR3
The theming and grid sections of the plugin guide told a consumer to declare the capability's shape locally and called the shape an implementation detail rather than a public export. Both are now published at the keys they are read from, so the sections show the import instead and describe what each contract carries rather than restating it in a block that could drift from the declarations it copies. The dialogs and config sections are untouched: their contracts are not published yet, and change 0031 rewrites them when they are. The guide also gains the one requirement raw TypeScript under a `types` condition places on a consumer, which the packed-consumer fixture has always proved and no document stated: `moduleResolution` must read an `exports` map and `allowImportingTsExtensions` must be on, or none of the subpaths resolve. The bundled-plugin import rule in the guide, in `AGENTS.md`, and in `REVIEW.md` each named `@fx/tx/plugin` as the only contract a plugin imports type-only. Each now names the published capability contracts beside it, and says what a published specifier is for between two bundled plugins: it is the one way one may name another's vocabulary, because it is erased and shares no runtime module graph, while a relative path into another plugin's directory stays rejected. Change 0030 is complete, so its task list, its status, and its entries in the documentation index are checked off and flipped, and the status notes in the plugin-system, theming, and grid specifications stop describing the grid contract as unpublished and the consumers as carrying local copies. Change 0031 stays draft; the dialogs and config keys remain bare. Claude-Session: https://claude.ai/code/session_0165ZoEriyA7L7iLAG81AkR3
…ually need The paragraph added with the guide's rewrite got both halves of its requirement wrong, and a reader following it would have hit either a compiler error or an unnecessary restriction. `node16` and `nodenext` were named as working alternatives with no mention of what TypeScript asks of them. Verified against the packed tarball: each needs its matching `module` — a mismatch is rejected with TS5110 before anything is checked — and needs the importing file to be an ES module, or a type-only import of this ESM package from a CommonJS file fails with TS1541. Both conditions are now stated, with `bundler` shown as the mode that needs nothing beside it. `allowImportingTsExtensions` was stated as required and is not. A consumer imports `@fx/tx/grid` rather than a path into it, and TypeScript follows the contracts' own `.ts` imports without the option; all four subpaths type check against the installed tarball without it, under `bundler` and under `node16`. Requiring it was not merely redundant — TypeScript accepts it only alongside `noEmit` or `emitDeclarationOnly`, so the guide was excluding every consumer that emits its own JavaScript or declarations. Change 0030's open question on emitted declarations said the same thing and is corrected with it. The consumer fixture now pins both claims rather than leaving them to rot. It type checks the module importing all four subpaths under exactly the settings the guide promises, with no `allowImportingTsExtensions`, and again under `node16` with its matching `module`. The whole-consumer project keeps that option, because the plugin sources it copies in name one another by `.ts` path — which is a property of how that consumer writes its own imports, not something these contracts ask for, and separating the projects is what makes the difference checkable. Claude-Session: https://claude.ai/code/session_0165ZoEriyA7L7iLAG81AkR3
There was a problem hiding this comment.
🟡 Changes recommended
The Change 0030 document is marked complete but still contains “today” language describing the pre-publication grid state, which should be updated for consistency.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Publishes the grid capability contract as a types-only public subpath (@fx/tx/grid) and renames the registry key to match, then updates bundled consumers/tests/demo/docs to import the published contracts instead of restating structural copies—closing out Change 0030’s remaining work.
Changes:
- Add
./gridexportsentry and packagefilesinclusion forplugins/grid/contract.ts, and rename the grid capability key to@fx/tx/grid. - Replace local structural copies of the grid/theming contracts across bundled consumers, demo, and tests with type-only imports from
@fx/tx/grid,@fx/tx/theme, and@fx/tx/theme-override. - Update specs/guides/review docs to document published-contract usage and the consumer TypeScript configuration matrix.
File summaries
| File | Description |
|---|---|
| test/theme-plugin.test.ts | Switches theme-related structural copies to published contract imports (@fx/tx/theme, @fx/tx/theme-override). |
| test/plugin-consumer.test.ts | Extends packed-file assertions and consumer fixture to import/type-check @fx/tx/grid and validates consumer tsconfig matrices. |
| test/plugin-boundary.test.ts | Adds @fx/tx/grid to the published subpath set enforced by boundary rules. |
| test/grid-select.test.ts | Types grid capability via @fx/tx/grid and updates registry reads to the new key. |
| test/grid-plugin.test.ts | Types grid capability/request/stream via @fx/tx/grid and updates registry reads to the new key. |
| test/grid-geometry.test.ts | Imports Row from the published grid contract instead of local types. |
| test/grid-cells.test.ts | Imports Row from the published grid contract instead of local types. |
| test/demo.test.ts | Updates demo grid stub types to use @fx/tx/grid and registers under the new key. |
| test/demo-scenarios.test.ts | Updates demo scenario types to import grid vocabulary from @fx/tx/grid. |
| REVIEW.md | Updates plugin review checklist to include type-only imports of published capability contracts (incl. grid). |
| plugins/grid/theme.ts | Removes restated theming vocabulary; uses published theming contract types for lookup typing. |
| plugins/grid/select.ts | Swaps internal grid type imports to come from the published-adjacent contract.ts. |
| plugins/grid/render.ts | Uses published theming contract types and grid contract request/stream types. |
| plugins/grid/index.ts | Renames capability registration key to @fx/tx/grid and types provider against the published contract. |
| plugins/grid/geometry.ts | Uses published theming contract variable types and grid contract row types. |
| plugins/grid/contract.ts | Re-frames as the published grid contract (types-only) and imports ThemeVariable from @fx/tx/theme. |
| plugins/grid/cells.ts | Uses published theming contract variable types and grid contract cell/row types. |
| plugins/dialogs/theme.ts | Removes restated theming vocabulary; uses published theming contract types for lookup typing. |
| plugins/dialogs/frame.ts | Switches theme-related types to import from @fx/tx/theme. |
| plugins/dialogs/columns.ts | Switches ThemeVariable typing to import from @fx/tx/theme. |
| package.json | Adds exports["./grid"] (types-only) and includes plugins/grid/contract.ts in files. |
| docs/specs/theming/index.md | Updates status text to reflect Change 0030 fully landed for theming consumers/docs. |
| docs/specs/plugin-system/index.md | Updates Published Capability Contracts status to include grid publication and consumer migrations. |
| docs/specs/grid/index.md | Updates status text to reflect grid contract publication/key rename now landed. |
| docs/manual/plugins.md | Updates consumer guidance to import published contracts and documents required TypeScript resolution settings. |
| docs/index.yml | Marks Change 0030 as complete. |
| docs/index.md | Marks Change 0030 as complete in the changes table. |
| docs/changes/0030-publish-bundled-capability-contracts.md | Marks the change doc complete and checks off remaining tasks/open-questions notes. |
| demo/scenarios.ts | Replaces restated grid/theming vocab with imports from published contracts; keeps dialogs structural copy pending 0031. |
| demo/index.ts | Switches grid capability typing and registry read to @fx/tx/grid. |
| AGENTS.md | Updates contributor guidance to allow type-only imports of published capability contracts (incl. grid). |
Review details
- Files reviewed: 31/31 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently align the grid registry key with a types-only published contract and update all shown consumers/tests/docs accordingly without introducing verified defects.
Review details
- Files reviewed: 31/31 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Publishes the grid capability's contract at
@fx/tx/grid, moves every bundled consumer, the demo, and the capability tests off their restated copies of the theming and grid shapes, and documents what a consumer actually needs to import a raw-TypeScripttypestarget.This completes docs/changes/0030-publish-bundled-capability-contracts.md — its remaining two task groups — so the change document and its
docs/index.ymlentry flip tocompletehere. Change 0031, which publishes the dialogs and config contracts, staysdraft; those two registry keys deliberately remain bare until it lands, because a key must never name a specifier that publishes nothing.What this does
Publishes the grid contract over the theming one.
plugins/grid/contract.tstakesThemeVariablefrom@fx/tx/themerather than restating it — through the published specifier, not a relative path into../theme/, which is exactly the escapetest/plugin-boundary.test.tsexists to forbid. The published path shares no runtime graph at all, because it is erased.plugins/grid/theme.tsloses its copy of the theme vocabulary and keeps only its capability lookup, and thegridkey becomes@fx/tx/gridin the same commit that publishes the contract.Retires the restated copies.
plugins/dialogs/theme.ts,demo/scenarios.ts, and the theming and grid test suites now import the published contracts instead of declaring their own. Nothing checked that those copies agreed with the provider; now the compiler does.Corrects the documented consumer configuration. The local Codex review disproved two claims this PR was about to ship, and both are fixed — see below.
Test plan
bun run checkpasses — biome clean across 91 files,tscclean, 1140 pass / 0 fail across 30 files, 100% functions/lines on production sources, build succeeds.@fx/tx/plugin,@fx/tx/grid,@fx/tx/theme,@fx/tx/theme-override, andtest/plugin-boundary.test.tsderives it from theexportsmap and holds each to the type-only, no-runtime-load, and no-src/-import rules, plus "transpiles to nothing".test/plugins.test.tsproves each subpath —@fx/tx/gridincluded — cannot be imported at run time.test/plugin-consumer.test.tsnow type checks three projects against the packed tarball: the whole consumer, one containing only the module that imports all four subpaths under exactly the settings the guide promises, and one underNode16/node16.Verified consumer tsconfig matrix
Measured against the packed
fx-tx-1.6.1.tgzinstalled into a realnode_modules, importingGrid/Row,ThemeVariable,ThemeOverride, andPlugin:module/moduleResolutionallowImportingTsExtensionsPreserve/BundlerPreserve/BundlerNode16/node16TS1541Node16/node16NodeNext/nodenextnode16TS5110CommonJS/Node10TS5108Preserve/Bundler, JS emit onNodeNext/nodenext, declaration emit onBundler, emit onTS5096Two things follow, and both were wrong in the first draft of this PR:
allowImportingTsExtensionsis not required and is no longer documented as such. A consumer imports@fx/tx/grid, not a.tspath, and TypeScript resolves the package's internal.tstype imports without it. Requiring it would have forced every consumer intonoEmit/emitDeclarationOnly, excluding ordinarytscemit workflows — the last row above. The guide now mentions it only for a consumer whose own sources import by.tspath, and states that cost. The claim in the change document's Open Questions has been corrected in the same PR that marks that document complete, rather than shipping a requirement this PR disproved.node16/nodenextneed two conditions the first draft omitted: a matchingmodulesetting, and an importer interpreted as an ES module. Both are now documented, and legacynoderesolution is called out as ignoringexportsentirely.The consumer fixture's own need for the flag came from the marketplace plugin sources it copies in, which import by
.tspath — not from the contracts. Splitting it into separate projects is what makes that distinction checkable rather than conflated.Notes
The status notes in
docs/specs/plugin-system/index.md,docs/specs/theming/index.md, anddocs/specs/grid/index.mdare updated to match. The dialogs and config notes are deliberately untouched — they remain accurate until change 0031 lands.Spec: docs/specs/plugin-system/index.md § Published Capability Contracts, docs/specs/grid/, docs/specs/theming/
Change: docs/changes/0030-publish-bundled-capability-contracts.md (completed by this PR)
https://claude.ai/code/session_0165ZoEriyA7L7iLAG81AkR3