Repository navigation
chore(Tokenizer): remove experimental.presentation; presentation is the only spelling - #9836
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
/approve-spec 97fd425 |
|
AI review status for this pull request.
|
There was a problem hiding this comment.
Removing Tokenizer's experimental spelling is covered by the release policy's experimental-change rule (AST-017 FR22) and the approved Tokenizer record. One change is needed before this can merge:
- Restore the
promote-tokenizer-presentationcodemod, with its TODO reworded because the old spelling no longer works, and add a CLI Changeset that names it, because AST-017 FR23 still requires a codemod when an API moves out of theexperimentalobject and the rewrite is mechanical, andexperimental={{presentation}}shipped in@astryxdesign/core@0.7.7; if owners agree that FR22's softer rule should govern, amend FR23 instead.
Without the codemod, typed callers of the 0.7.7 spelling get a compile error with no astryx upgrade step. Untyped callers silently get the 'adaptive' default, and experimental="[object Object]" lands on the input.
[Automated review]
| meta: promoteTokenizerPresentationMeta, | ||
| }, | ||
| ]; | ||
| export default []; |
There was a problem hiding this comment.
experimental={{presentation}} shipped in @astryxdesign/core@0.7.7, and AST-017 FR23 still requires a codemod when an API moves out of the experimental object and the rewrite is mechanical. Please keep promote-tokenizer-presentation registered here, reword its TODO (the old spelling no longer works), and add a CLI Changeset for it. Or, if owners agree, amend FR23.
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Tokenizer · Storybook | 71.4 kB (−64 B) · 17.1 kB CSS (0) | 2,116 (−12) | ✓ · ✓ · ✓ |
| PowerSearch | 195.1 kB (−91 B) · 21.6 kB CSS (0) | — | — · — · — |
Package size:
- @astryxdesign/cli 3.32 MB packed (−1.8 kB); no new dependencies
- @astryxdesign/core 6.26 MB packed (−941 B); no new dependencies
API changes: core +0 · ~0 · −1
Committed API report behind this build: core 1 row; run pnpm api:report.
1 API change (advisory)
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| − | core |
. · Components · component Tokenizer |
prop experimental?: { presentation?: TokenizerPresentation } @experimental |
<Tokenizer experimental={…} /> |
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
97fd425 to
93731ce
Compare
|
/approve-spec 93731ce |
There was a problem hiding this comment.
Removing Tokenizer's experimental spelling is covered by the release policy (AST-017 FR22 and FR23) and the approved Tokenizer record, and the kept codemod gives 0.7.7 callers an astryx upgrade step. Two wording fixes are needed before this can merge:
- The description says untyped callers get React's unknown-prop warning, but none fires: the field silently ignores
experimental, renders the'adaptive'default and putsexperimental="[object Object]"on the input, so please describe that instead. - The core Changeset says "
astryx upgraderewrites static uses", but the codemod leaves a Tokenizer that spreads props unchanged with a TODO (its own test covers both spread orders), so please add that caveat, for example "rewrites static uses and leaves a TODO where it cannot prove the rewrite".
Optional: the codemod's TODO ends with "(a spread may also set presentation)" even where no spread is involved, such as experimental={options}.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [experimental] `experimental.presentation` is removed from Tokenizer, together with its `experimental` prop; use the stable `presentation` prop, which takes the same values. `astryx upgrade` rewrites static uses. |
There was a problem hiding this comment.
astryx upgrade leaves a Tokenizer that spreads props unchanged with a TODO (the codemod test covers both spread orders), so "rewrites static uses" claims more than it does. Suggested: "astryx upgrade rewrites static uses and leaves a TODO where it cannot prove the rewrite, such as on a Tokenizer that spreads props."
| 'could not prove the rewrite. '; | ||
| ' TODO(astryx upgrade): experimental.presentation was removed in favor of ' + | ||
| 'presentation. Move the value to presentation; the codemod left this one ' + | ||
| 'unchanged because it could not prove the rewrite (a spread may also set ' + |
There was a problem hiding this comment.
Non-blocking: this TODO is also written for experimental={options} and for a Tokenizer that already passes presentation, where no spread is involved, so the "(a spread may also set presentation)" reason misleads there. Drop the parenthetical or give each case its own reason.
93731ce to
5132931
Compare
|
/approve-spec 5132931 |
There was a problem hiding this comment.
Approval is recommended. Removing Tokenizer's experimental spelling is covered by the release policy (AST-017 FR22 and FR23) and by the Tokenizer record approved at this head, and both earlier wording fixes are in.
Optional, in this pull request or a follow-up:
.changeset/cli-discover-lab-mobile-sheets-removed.mdstill says the removed Lab components' touch flows are "Core's Tokenizer and PowerSearchexperimental.presentationsheets". The same release removes that spelling, so consider "presentationsheets".
[Automated review]
…ved experimental prop #9808 added the test with experimental.presentation; #9836 removed that prop. Each was green alone; together main typecheck fails (TS2769). Same change as #9847, folded in here so one pull request turns main green. Co-authored-by: Xinzi Zhou <2252473+imdreamrunner@users.noreply.github.com>
…tion, and the recorded module resolutions follow core (#9851) * test(cli): the recorded module resolutions follow useLightbox's doc and drop the deleted usePressPin #9752 gave useLightbox its own doc; #9756 deleted the internal Layer/usePressPin.ts. Each was green alone because PR checks skip this fixture test; main test-node fails on both lines. * test(Tokenizer): the sheet-row test passes presentation, not the removed experimental prop #9808 added the test with experimental.presentation; #9836 removed that prop. Each was green alone; together main typecheck fails (TS2769). Same change as #9847, folded in here so one pull request turns main green. Co-authored-by: Xinzi Zhou <2252473+imdreamrunner@users.noreply.github.com>
Primary intent
Remove Tokenizer's
experimentalprop now thatpresentationis a stable prop (#9796). The prop held onlypresentation. The stagedpromote-tokenizer-presentationcodemod stays and now targets the removed spelling, so callers of 0.7.7 get anastryx upgradestep. The PR removes an experimental API, so it is neither a new feature nor a fix, and maintenance does not fit either.User or maintainer impact
experimental={{presentation: x}}are affected. That spelling shipped in 0.7.7, a few hours before the stable prop merged. They passpresentation={x}, which takes the same values and behaves the same, or runastryx upgrade. Typed callers get a compile error until they move. For untyped callers, the field ignoresexperimental, renders the'adaptive'default, and putsexperimental="[object Object]"on the input.presentationprop, with no deprecated alias. The removal note names the replacement andastryx upgrade.Problem and solution fit
spec:AST-017/FR22: removing an explicitly experimental surface is an[experimental]patch change, and "a compatibility alias is optional". The spelling did ship in 0.7.7, and the rewrite topresentationis mechanical, so FR23 still asks for the codemod. It stays.Authority
spec:AST-017/FR22(experimental removal) and FR23 (promotion, and the codemod for a mechanical rewrite).Change and evidence
Tokenizer.tsx. Theexperimentalprop, its resolution path and the precedence warning are removed.presentationdefaults to'adaptive'directly. The wider-refoverload stays, because the stable prop's sheet presentation still puts the trigger button inref.Tokenizer.doc.mjs. Theexperimentalentries are removed (English, Chinese and dense).promote-tokenizer-presentationstays registered intransforms/next, for the removed spelling. It rewrites only spread-free call sites. Any spread on the element, before or afterexperimental, leaves the call unchanged with a TODO, because a spread can carry its ownpresentation, also asundefined. Each TODO starts "experimental.presentation was removed in favor of presentation." and then names its case. A spread: "a spread may also set presentation".experimental={options}: "the value is not a literal object". A spread or computed key inside the object: "the object has a spread or a computed key". A repeated key: "the object sets presentation more than once". A Tokenizer that already passespresentation: "presentation is already set; remove experimental.presentation". Its tests cover both spread orders, and a second run changes nothing.[experimental]patch Changeset for@astryxdesign/core: "experimental.presentationis removed from Tokenizer, together with itsexperimentalprop; use the stablepresentationprop, which takes the same values.astryx upgraderewrites static uses and leaves a TODO where it cannot prove the rewrite, such as on a Tokenizer that spreads props."[feat]Changeset no longer promises an alias.[chore]cli Changeset.component:Tokenizerdrops the deprecated-alias sentences from the summary table, Compatibility, Public concepts, FR16, the verification row and DEC-3. Compatibility states that the experimental spelling is removed under FR22, and thatastryx upgraderewrites static uses and leaves a TODO where it cannot prove the rewrite (FR23).system_specsnow listsspec:AST-017/FR22in place of FR21.api-report.md. Only the removedexperimentalline changes. A staleCodeBlockstartLineNumberline thatapi:reportregenerates on main is left out.grep -rn "experimental={{presentation\|experimental\.presentation" packages appsfinds hits only in the codemod and its tests. There are none in Core, blocks, templates or stories.Scope
Testing
internal/, adopters,slotPresence,touchTriggerTargets, the i18n PowerSearch end-to-end test, the CLI codemods, the CLI golden reads, experimental isolation and the single-bundle test: 172 files and 3,385 tests pass.pnpm typecheckpasses for every workspace.check:changesets,check:sync,check:knowledgeandsync-exports --checkpass. Changeset coverage passes; this PR ships to@astryxdesign/coreand@astryxdesign/cli, and a Changeset names each.codemod-verify.mjspasses, because no consumer file changes: the stories already usepresentation.Writing rubric results
docs/specs/components/Tokenizer/Tokenizer.spec.md: