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

UI: Upgrade to Emotion 11 #13300

Closed
wants to merge 7 commits into from
Closed

UI: Upgrade to Emotion 11 #13300

wants to merge 7 commits into from

Conversation

tooppaaa
Copy link
Contributor

@tooppaaa tooppaaa commented Nov 26, 2020

Issue: #13145

What I did

Upgrade to emotion 11

How to test

  • chroma ?

@shilman
Copy link
Member

shilman commented Nov 27, 2020

@tooppaaa @ndelangen what are the implications to users? Is this a 6.2 change or a 7.0 change?

@ndelangen
Copy link
Member

Hmm, should not impact users at all AFAIK

@osdiab
Copy link
Contributor

osdiab commented Jun 22, 2021

Right now I'm not sure the root cause but when I use the css prop with a style I've defined, that works fine on my actual website, it causes Storybook to apparently infinite loop and crash in my browser. Giving up on trying to make it work for now unless someone has a suggestion on what might be going on lol happy to provide a code sample when I'm in front of my computer.

@darleendenno darleendenno added this to the 7.0 milestone Jul 12, 2021
costasovo added a commit to mailpoet/mailpoet that referenced this pull request Aug 11, 2021
Storybook uses @emotion packages v10 and @wordpress/components
require v11. Storybook works with version 11 but since there were some merges
in @emotion packages we need to set some aliases and also make sure correct versions
are installed on top level in node_modules.
See storybookjs/storybook#13300 (comment)
This workaround was done based on a workaround applied in the Gutenberg project
WordPress/gutenberg@bc072fd
[MAILPOET-3654]
costasovo added a commit to mailpoet/mailpoet that referenced this pull request Aug 24, 2021
Storybook uses @emotion packages v10 and @wordpress/components
require v11. Storybook works with version 11 but since there were some merges
in @emotion packages we need to set some aliases and also make sure correct versions
are installed on top level in node_modules.
See storybookjs/storybook#13300 (comment)
This workaround was done based on a workaround applied in the Gutenberg project
WordPress/gutenberg@bc072fd
[MAILPOET-3654]
veljkho pushed a commit to mailpoet/mailpoet that referenced this pull request Aug 30, 2021
Storybook uses @emotion packages v10 and @wordpress/components
require v11. Storybook works with version 11 but since there were some merges
in @emotion packages we need to set some aliases and also make sure correct versions
are installed on top level in node_modules.
See storybookjs/storybook#13300 (comment)
This workaround was done based on a workaround applied in the Gutenberg project
WordPress/gutenberg@bc072fd
[MAILPOET-3654]
@shilman shilman mentioned this pull request Sep 29, 2021
@vtereshyn
Copy link

Any updates here?

@shilman
Copy link
Member

shilman commented Oct 21, 2021

@vtereshyn AFAIK we are blocked until SB7.0 unless people have other ideas

@AmirTugi
Copy link

Hey hey!
I would really appreciate promoting this as this breaks my usage of Chakra (uses @emotion/react ^11) and some storybook addons which are using @emotion/core ^10 and is throwing an uncaught error that I must use @emotion/react.

I would really love to see this merging into storybook soon! 🥳 🙏🏼

@karol-f
Copy link

karol-f commented Oct 27, 2021

@AmirTugi using storybook webpack config (in main.js) with Emotion aliases (it's called fallback in Webpack 5) won't solve your issue? It does in my setup:

webpackFinal: async (config) => {
	config.resolve.fallback = {
		// TODO: Remove once Storybook supports Emotion 11.
		'@emotion/styled': require.resolve('@emotion/styled'),
		'@emotion/core': require.resolve('@emotion/react'),
		'@emotion-theming': require.resolve('@emotion/react'),
		'@emotion/react': require.resolve('@emotion/react'),
	};
...

@levymetal
Copy link

Thanks for the tip @karol-f. We had to do one additional thing to get it working; we had to add @emotion/sheet@^1.0.3 to our package. Without this, @emotion/sheet gets hoisted incorrectly; @emotion/sheet@0.9.4 from @storybook#theming#@emotion#core#@emotion#cache gets hoisted to @emotion/sheet, and Emotion 11 pulls that instead of @emotion/sheet@^1.0.3 throwing a TypeError: cache.sheet.hydrate is not a function. Manually adding @emotion/sheet@^1.0.3 to the package hoists it to @emotion/sheet. Also, this only seems to happen with yarn and not npm.

This looks like an issue with yarn or Emotion, not Storybook. Tracking it on a similar (closed) issue: emotion-js/emotion#1974 (comment). Repro here: https://github.com/levymetal/emotion-11-conflict-repro

@Andarist
Copy link
Contributor

Andarist commented Nov 8, 2021

@levymetal you have misconfigured your webpack's config.resolve.modules. Try this patch and see that it works then:

diff --git a/webpack.config.js b/webpack.config.js
index 74479f3..b2ba83a 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -3,7 +3,7 @@ const path = require("path");
 
 const paths = {
   src: path.join(__dirname, "src"),
-  nodeModules: path.join(__dirname, "node_modules"),
+  nodeModules: path.join(".", "node_modules"),
 };
 
 module.exports = {
@@ -32,7 +32,7 @@ module.exports = {
       {
         test: /\.(js|jsx)$/,
         include: [paths.src],
-        exclude: [paths.nodeModules],
+        exclude: /\/node_modules\//,
         use: ["babel-loader"],
       },
     ],

See in the webpack docs how relative and absolute paths are treated differently in this context: https://webpack.js.org/configuration/resolve/#resolvemodules . Basically, you have made webpack to load @emotion/sheet from your root node_modules - that's why it was loading it "incorrectly".

@levymetal
Copy link

@Andarist I can't thank you enough for your response. That resolves the issue not just in the repro, but also the actual project I'm working on. Our webpack config has been misconfigured like this for over 5 years 💀. Now many of the other issues we've had suddenly make sense.

@stale
Copy link

stale bot commented Jan 9, 2022

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Jan 9, 2022
@dantman
Copy link

dantman commented Jan 9, 2022

This is blocked till the next major, not inactive. And it's a PR with actual code contributed, not an issue. It shouldn't be free for a bot to carelessly close.

We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

This message looks poorly written for whatever rule is triggering it. The bot is telling users it's going to close a PR and they should contribute a PR instead.

@stale stale bot removed the inactive label Jan 9, 2022
@jorisw
Copy link
Contributor

jorisw commented Jan 9, 2022

I've always thought stalebots are a stupid thing.

Just because an issue was sufficiently documented and therefore received no new messages, it's somehow no longer relevant?

And this after only 30 days?

@mrlubos
Copy link

mrlubos commented Jan 17, 2022

@bebraw Thanks, this got me most of the way there. I also had to do the same to managerWebpack as that contains aliases referencing the v10 packages and would give me runtime errors without it. This only became apparent to me when I cleared node_modules/.cache/storybook.

Also found v10 references in the default babel config which I needed to replace in order to get the CSS prop to work at runtime.

Finally I also needed to install @emotion/styled as a dev dependency to ensure I had v11 in node_modules and not the v10 that gets installed for Storybook.

In the end this setup is now working for me (Emotion 11, Storybook 6.1.18, Yarn Workspaces).

.storybook/main.js

// Location of root node_modules
const modulesDir = path.join(__dirname, '../../../../node_modules');

const updateEmotionAliases = (config) => ({
  ...config,
  resolve: {
    ...config.resolve,
    alias: {
      ...config.resolve.alias,
      '@emotion/core': path.join(modulesDir, '@emotion/react'),
      '@emotion/styled': path.join(modulesDir, '@emotion/styled'),
      '@emotion/styled-base': path.join(modulesDir, '@emotion/styled'),
      'emotion-theming': path.join(modulesDir, '@emotion/react'),
    },
  },
});

module.exports = {
  // ...
  managerWebpack: updateEmotionAliases,
  webpackFinal: updateEmotionAliases,
  babel: (config) => {
    const getEntryIndexByName = (type, name) => {
      return config[type].findIndex((entry) => {
        const entryName = Array.isArray(entry) ? entry[0] : entry;
        return entryName.includes(name);
      });
    };

    // Replace reference to v10 of the Babel plugin to v11.
    const emotionPluginIndex = getEntryIndexByName('plugins', 'babel-plugin-emotion');
    config.plugins[emotionPluginIndex] = require.resolve('@emotion/babel-plugin');

    // Storybook's Babel config is already configured to use the new JSX runtime.
    // We just need to point it to Emotion's version.
    // https://emotion.sh/docs/css-prop#babel-preset
    const presetReactIndex = getEntryIndexByName('presets', '@babel/preset-react');
    config.presets[presetReactIndex][1].importSource = '@emotion/react';

    return config;
  },
  // ...
};

Hey everyone! I used to use this excellent quote trick, but found that it's no longer necessary after disabling emotionAlias like this in your main.js:

features: {
  emotionAlias: false
}

@ronaldruzicka
Copy link

@mrlubos thanks a lot! I spent almost 2 days figuring this thing out.

@ndelangen ndelangen mentioned this pull request Mar 30, 2022
3 tasks
@Andarist
Copy link
Contributor

This is being superseded by #17640

@Andarist Andarist closed this Mar 30, 2022
@stof stof deleted the feature/emotion11 branch May 25, 2022 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE core maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet