Repository navigation
feat: kbd and hotkeys codemods - #3288
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 4 Skipped Deployments
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
🦋 Changeset detectedLatest commit: 8187790 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
colepeters
left a comment
colepeters
left a comment
There was a problem hiding this comment.
One very minor nit, and trusting that moveChildrenToAttribute functions as per the test, otherwise LGTM!
| // UI-CODEMOD TODO: Move children into the text prop | ||
| <Badge><Text>{label}</Text></Badge> | ||
| `, | ||
| 'warns on JSX element child onto the attribute', |
There was a problem hiding this comment.
Think the description here might be missing a word?
There was a problem hiding this comment.
I think I made a copy and paste error here because it should say "warns on JSX element child when stringOnly is true". Updated that.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8187790. Configure here.
| const element = parentNode | ||
| const children = ((element.children ?? []) as JSXElementChild[]).filter( | ||
| (child) => !(child.type === 'JSXText' && child.value.trim() === ''), | ||
| ) |
There was a problem hiding this comment.
Whitespace dropped between moved children
Low Severity
moveChildrenToAttribute filters out every whitespace-only JSXText node before moving children. That is correct for indentation, but when stringOnly is false and several children are wrapped in a fragment, significant spaces between expressions are dropped, so {first} {last} becomes {first}{last}.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8187790. Configure here.
Description
This PR adds codemods for KBD and Hotkeys. For KBD, it moves
childrentotext(if the value is a string, otherwise it warns), removessizewhich doesn't currently do anything, and transforms the rest of the props to inline styles. For Hotkeys, we've deprecated everything butkeysso it removes all those. I didn't try to transform them because they only really make sense if they can be passed into KBD. If a user wants different styling, they'll have to handle that on their own.It also adds a new helper that can move
childreninto a new attribute. It can either do allchildrencontent or can setstringOnlyflag to only transform strings and warn about anything else. I swapped it out in Badge too since it seemed like it might be useful behavior there.What to review
Make sure the codemods are behaving as described. For the new
moveChildrenToAttributefunction, I had the robot do most of that work since interacting with jscodeshift gets complicated otherwise.Testing
I wrote tests for the new util and both codemods. I might also try this in Studio once it's published.
Note
Low Risk
Changes are limited to the ui-codemod package and migration tooling; runtime UI behavior is unaffected aside from automated codemod output on consumer codebases.
Overview
Adds latest CLI commands and jscodeshift transforms for
KBDandHotkeys, with new package exports and a patch changeset.The KBD codemod rewrites imports, drops
size, mapsfontSize/padding/radiusto inlinestyletokens, and moves static string children totext(non-string children get a UI-CODEMOD TODO). StyledKBDwrappers are handled via the same migration path.The Hotkeys codemod rewrites imports and strips deprecated layout props (
fontSize,padding,radius,gap,space) without trying to remap them to styles.Shared tooling: new
moveChildrenToAttributehelper (withgetStaticStringFromChildandgetAttributeValueAST support) powers children→**text** migration; the Badge transform now uses it withstringOnly: true, so plain string badge labels auto-migrate instead of only warning.Reviewed by Cursor Bugbot for commit 8187790. Bugbot is set up for automated code reviews on this repo. Configure here.