Repository navigation
fix(cli): promote-tokenizer-presentation puts presentation before a spread - #9826
imdreamrunner wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
/approve-spec 20a6a01 |
|
AI review status for this pull request.
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes. Moving presentation before the first spread fixes the case where the spread sets a value, but it breaks the case where the spread's presentation is undefined. For a spread the codemod cannot read, no position is provable.
promote-tokenizer-presentation.mjsrewrites<Tokenizer {...args} experimental={{presentation: 'bottom-sheet'}} />to<Tokenizer presentation="bottom-sheet" {...args} />with no TODO, yet whenargshas its ownpresentation: undefinedthe spread now overwrites the moved value and Tokenizer falls back to theadaptivedefault (the inline field on a desktop) where the original call rendered the bottom sheet, so a spread beforeexperimentalshould keep the deprecated spelling with the TODO, asspec:AST-017FR29c and FR29d ask for spreads a codemod cannot prove.
Then update the spread test to expect the TODO, and make the header comment, meta.description and the Changeset match.
[Automated review]
| let insertAt = value.properties.length === 0 ? index : index + 1; | ||
| if (firstSpread !== -1) { | ||
| insertAt = firstSpread; | ||
| } | ||
| opening.attributes.splice(insertAt, 0, presentation); |
There was a problem hiding this comment.
Before the spread, a spread with its own presentation: undefined overwrites the moved value. Tokenizer then resolves presentationProp ?? experimental?.presentation ?? 'adaptive' to 'adaptive', because experimental is gone, where the original call rendered the bottom sheet. After the spread, a defined presentation loses instead. Neither position is provable for an unknown spread, so please treat every spread as unprovable in isProvable (drop the index > attributes.indexOf(prop) condition) and keep the deprecated spelling with the TODO.
| ); | ||
| expect(output).toContain( | ||
| '<Tokenizer {...args} label="Tags" presentation="bottom-sheet" />', | ||
| '<Tokenizer presentation="bottom-sheet" {...args} label="Tags" />', |
There was a problem hiding this comment.
This pins a TODO-free rewrite past a spread whose contents the codemod cannot see. With the spread treated as unprovable, this case should expect the TODO and the kept experimental={{presentation: 'bottom-sheet'}}.
PR Analysis Report🔎 Preview ·
|
|
Closing: the staged |
User impact
Take a builder who runs
astryx upgradeon code that spreads props into a Tokenizer and also passesexperimental={{presentation: x}}after the spread, for example<Tokenizer {...props} experimental={{presentation: 'bottom-sheet'}} />. Ifpropscarries its ownpresentation, that value wins today: the top-level prop wins over the experimental one. The stagedpromote-tokenizer-presentationcodemod (#9796) writespresentation="bottom-sheet"whereexperimentalwas, after the spread. The moved value then replaces the spread's, so the surface that lost starts to win. After this PR, the codemod keeps the result the same.Problem and solution fit
experimental. In JSX, a later attribute replaces an earlier spread's key.spec:AST-017/FR29c). Here it silently changes which presentation renders.astryx upgrade --applyruns without review of each call site, so the change would reach production unnoticed.The fix puts the stable attribute before the first spread. A
presentationin the spread then still wins, and without one the moved value applies. A spread afterexperimentalcan replace the wholeexperimentalobject, so the codemod still leaves that case to a person, with a TODO.Expected behavior and authority
spec:AST-017/FR29c: "A mechanical migration … preserves the old observable result as its baseline and reports every uncertain or blocked case."component:TokenizerDEC-3: when both are set, the top-levelpresentationwins overexperimental.presentation.Smallest restoration
The change is the insertion index in
promote-tokenizer-presentation.mjs, plus its header comment, one updated test and a Changeset. Nothing else changes. #9797 makes the same change in the PowerSearch codemod it adds.Evidence
<Tokenizer {...args} label="Tags" experimental={{presentation: 'bottom-sheet'}} />becomes<Tokenizer {...args} label="Tags" presentation="bottom-sheet" />, so apresentationinargsloses.<Tokenizer presentation="bottom-sheet" {...args} label="Tags" />.experimental. With a spread afterexperimental, the codemod still leaves a TODO.Scope
Testing
codemod-verify.mjspasses, because no consumer file changes.check:changesetsand Changeset coverage (it ships to@astryxdesign/cli, which the Changeset names) pass, and strict lint reports 0 errors.