Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty @keyframes declaration is removed in production mode #2392

Closed
michal-perlakowski opened this issue May 27, 2021 · 8 comments
Closed

Empty @keyframes declaration is removed in production mode #2392

michal-perlakowski opened this issue May 27, 2021 · 8 comments

Comments

@michal-perlakowski
Copy link

import { Global } from '@emotion/react';

function App() {
  return (
    <div>
      <Global
        styles={{
          '@keyframes test-animation': {},
        }}
      />
      <div
        id="test-el"
        style={{
          animationName: 'test-animation',
          animationDuration: '10ms',
        }}
        onAnimationStart={(event) => {
          console.log(event.animationName);
        }}
      />
    </div>
  );
}

export default App;

Expected behavior: 'test-animation' is logged in console. This works fine in development mode, but not in production.

Demo repository: https://github.com/michal-perlakowski/emotion-test

This is causing a bug in Material-UI.

Environment information:

  • react version: 17.0.2
  • @emotion/react version: 11.4.0
@Andarist
Copy link
Member

If a tree falls in a forest and no one is around to hear it, does it make a sound? If animation is not animating anything - did it even happen? 😉

Could you describe your use case?


In dev we are using a custom fork of the Stylis' rulesheet plugin which injects empty rules (from what I recall this mainly is done to make the cache key discoverable by @emotion/jest):

if (element.return) {
currentSheet.insert(element.return)
} else if (element.value && element.type !== COMMENT) {
// insert empty rule in non-production environments
// so @emotion/jest can grab `key` from the (JS)DOM for caches without any rules inserted yet
currentSheet.insert(`${element.value}{}`)
}

In this situation the element has an empty return but still has value that we use to inject this rule in development.

In production, we are using the official rulesheet plugin which skips over such elements:
https://github.com/thysultan/stylis.js/blob/e6843c373ebcbbfade25ebcc23f540ed8508da0a/src/Middleware.js#L31-L32

@michal-perlakowski
Copy link
Author

@Andarist Material-UI uses empty animations to detect autofill in text fields (see the linked issue).

@michal-perlakowski
Copy link
Author

Perhaps this should be fixed in Stylis - I opened an issue there: thysultan/stylis#265.

@Andarist
Copy link
Member

Material-UI uses empty animations to detect autofill in text fields (see the linked issue).

The linked issue has no context for this technique. Could you expand on that?

@michal-perlakowski
Copy link
Author

@Andarist You apply the animation to the :-webkit-autofill selector and add animationstart event handler to the input element. Here's a demo.

@oliviertassinari
Copy link

oliviertassinari commented May 31, 2021

There are two links of the approach from third-parties in mui/material-ui#14427 (comment).

@michal-perlakowski
Copy link
Author

@Andarist Stylis' owner hasn't replied to the issue and has been inactive on GitHub in general for a few months. Maybe we should fix this in emotion?

michal-perlakowski added a commit to michal-perlakowski/emotion that referenced this issue Dec 10, 2021
@Andarist
Copy link
Member

A new version of Stylis has been published - we are going to bump the declared dependency range in Emotion packages sometime later but you should be able to already leverage the fix by pruning Stylis from your lock file and reinstalling packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants