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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Update Emotion-related deps #18992

Closed
wants to merge 2 commits into from
Closed

Update Emotion-related deps #18992

wants to merge 2 commits into from

Conversation

Andarist
Copy link
Contributor

fixes #18656

I've finally fixed the issue with ignoring some of those ignore flags in Emotion - so upgrading to the new version of Emotion should fix your problems 馃

With this, changes from https://github.com/storybookjs/storybook/pull/18361/files could be reverted if you want
This PR also closes #18657

@Andarist Andarist mentioned this pull request Aug 22, 2022
Closed
@ndelangen
Merge branch 'next' into update-emotion-deps
183e08b 
# Conflicts:
#	code/addons/storyshots/storyshots-core/package.json
#	code/yarn.lock
@tmeasday
Copy link
Member

tmeasday commented Aug 26, 2022 •
edited

I'm getting these errors when I run a sandbox

yarn sandbox --template cra/default-ts

(likely the same issue as CI is having):

image

@ndelangen it looks like a manager bundling issue?

@tmeasday
Copy link
Member

tmeasday commented Aug 26, 2022 •
edited

PS when I run the sandbox on next (where it works fine), I don't see the original issue:

image

Any idea on why that might be @Andarist @ndelangen ?

@tmeasday
Copy link
Member

I also created this branch https://github.com/storybookjs/storybook/tree/revert-disable-emotion-unsafe-ssr-warnings where I reverted #18361 as you suggested @Andarist and I am not seeing the warnings in a sandbox either.

Which version of emotion resolved the issue? It looks like monorepo is currently bundling version 11.9.x, perhaps the fix is already in there?

@tmeasday
Copy link
Member

Confirmed I don't see the warning in docs mode in the latest 7.0 alphas.

@Andarist
Copy link
Contributor Author

This was fixed by @emotion/cache@11.10.2 (had to fix a regression in a follow-up release though: @emotion/cache@11.10.3). This depends on the rendered components - since the warning only gets printed when such "unsafe" styles are being rendered. Maybe you have not entered a screen that would trigger this?

I've confirmed now that the issue can be reproduced using this repro (on Storybook 6.5.x) after entering the Docs tab in Button stories.

This PR is worth merging anyway as it should be a safe upgrade. If you want to confirm that it actually fixed the problem then I would appreciate it if you could create a repro with the current 7 alpha that I could take a look at.

@ndelangen ndelangen mentioned this pull request Aug 26, 2022
Merged
@tmeasday
Copy link
Member

@Andarist I tried in a vanilla SB7 project and I don't see the same warnings. I'm not sure what would have fixed it though, if as you say the upstream problem wasn't fixed until 11.10.x. I agree we may as well merge this once CI is passing though.

I suspect @ndelangen is most likely to have the answers on both fronts.

@Andarist
Copy link
Contributor Author

This got superseded by #19029

@Andarist Andarist closed this Aug 30, 2022
@Andarist Andarist deleted the update-emotion-deps branch August 30, 2022 14:04
@tmeasday tmeasday mentioned this pull request Feb 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndelangen ndelangen ndelangen approved these changes

@tmeasday tmeasday Awaiting requested review from tmeasday

Assignees

@ndelangen ndelangen

Labels
dependencies
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Console error about server-side rendering on a Docs page
4 participants
@Andarist @tmeasday @ndelangen @shilman