Skip to content

Conversation

@siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Mar 7, 2024

Root cause

Passing empty args ([]) to emotion's serializeStyles is not allowed.

Full error:

1) Pigment CSS - keyframes
       basics:
     Expected test not to call console.error() but instead received 1 calls.
If the warning is expected, test for it explicitly by using the toErrorDev() matcher.
Test location:
  /Users/siriwatknp/Personal-Repos/material-ui/packages/pigment-react/tests/keyframes/keyframes.test.ts 
console.error message #1:
  You have illegal escape sequence in your template literal, most likely inside content's property value.
Because you write your CSS inside a JavaScript string you actually have to do double escaping, so for example "content: '\00d7';" should become "content: '\\00d7';".
You can read more about this here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#ES2018_revision_of_illegal_escape_sequences
Stack:
    at Object.serializeStyles (/Users/siriwatknp/Personal-Repos/material-ui/node_modules/.pnpm/@[email protected]/node_modules/@emotion/serialize/dist/emotion-serialize.cjs.dev.js:278:17)
    at KeyframesProcessor.generateArtifacts (/Users/siriwatknp/Personal-Repos/material-ui/packages/pigment-react/src/processors/keyframes.ts:97:6)
    at KeyframesProcessor.handleTemplate (/Users/siriwatknp/Personal-Repos/material-ui/packages/pigment-react/src/processors/keyframes.ts:92:7)
    at KeyframesProcessor.build (/Users/siriwatknp/Personal-Repos/material-ui/packages/pigment-react/src/processors/keyframes.ts:51:5)

The rest of the changes are splitting tests into small cases


@mui-bot
Copy link

mui-bot commented Mar 7, 2024

Netlify deploy preview

https://deploy-preview-41395--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 5f36e63

@danilo-leal danilo-leal added the package: pigment-css Specific to Pigment CSS. label Mar 7, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 8, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 8, 2024
Comment on lines 37 to 43
const [, ...callParamsRest] = callParams;

callParamsRest.flat().forEach((item) => {
if ('kind' in item) {
this.dependencies.push(item);
}
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow the styled implementation.

Comment on lines +105 to +107
args.length > 0
? [styleObjOrTaggged as Interpolation<{}>, ...args]
: [styleObjOrTaggged as Interpolation<{}>],
Copy link
Member Author

@siriwatknp siriwatknp Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is required to prevent ILLEGAL_ESCAPE_SEQUENCE_ERROR from emotion because serializeStyles assumes that [] is an expression.

@siriwatknp siriwatknp marked this pull request as ready for review March 8, 2024 11:30
@siriwatknp siriwatknp requested a review from brijeshb42 March 8, 2024 11:30
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 8, 2024
@oliviertassinari oliviertassinari changed the title [pigment] Add a support for tagged template call [pigment-css] Add a support for tagged template call Mar 10, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 12, 2024
@siriwatknp siriwatknp enabled auto-merge (squash) March 12, 2024 07:11
@siriwatknp siriwatknp merged commit d93226a into mui:master Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: pigment-css Specific to Pigment CSS.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants