[codemod] Support keyframes in jss-to-styled - #49119
Open
stop1love1 wants to merge 1 commit into
Open
stop1love1 wants to merge 1 commit into
stop1love1 wants to merge 1 commit into
Conversation
`createClasses` read `prop.key.name` for every entry of the JSS style object. JSS declares keyframes with a string key, `'@Keyframes pulse'`, whose `name` is undefined, so the codemod threw `undefined does not match field "name"` and no part of the file was migrated. A keyframes entry is not a class: neither it nor the percentages it nests may be turned into a class or into a `& .${classes.x}` selector. Carry it over untouched, and drop the `$` from the animation names that reference it, since emotion refers to a keyframe by its bare name. Closes mui#30801
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
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.
Closes #30801
Problem
v5.0.0/jss-to-styledthrows on any style object that declares keyframes, and since the transform aborts, nothing in the file is migrated:createClasseswalks every entry of the style object and readsprop.key.name, assuming each one is a class name written as an identifier. JSS declares keyframes with a string key, soprop.key.nameisundefined, andj.identifier(undefined)throws.This matches what was established in the issue thread — @mnajdova asked whether keyframes were supported and @siriwatknp confirmed "the Codemod does not transform the keyframes at this point".
Solution
A keyframes entry is not a class, and — unlike
@media— the keys it nests (0%,from,to) are not class names either. So the whole property is carried over untouched, which is exactly what emotion expects in an object style. It is skipped in the three places that build class names: theclassesmap, the& .${classes.x}selector rewrite, and thewithStylesclasses mapping.One more step is needed for the output to actually work: JSS references a keyframe as
$pulse, emotion by its bare name. The$is dropped, but only for names that the same style object really declares via@keyframes, so unrelated strings are left alone.Deliberately out of scope
Other at-rules (
@media,@supports) hit the sameprop.key.namecrash, but they do nest class names, so skipping them would turn a loud crash into silently wrong output. They need their own handling and are left untouched here.Testing
A
bugs - #30801 keyframesfixture pair plus the idempotency test the other cases use. Onmasterthe new case fails with the crash above; with the fix both pass.Whole
@mui/codemodpackage:686 passedon this branch vs684 passedonmaster— the two new tests, no other change. Both runs show the same pre-existing failure,path-imports > should not leak imports between file transformations, which is order-sensitive and unrelated.eslintandprettier --check --ignore-path .lintignoreare clean on the touched files.