Skip to content

fix: emotion compatibility with latest storybook#5570

Merged
jtoar merged 6 commits intoredwoodjs:mainfrom
callingmedic911:fix/emotion-compat-with-latest-storybook-and-emotion
May 24, 2022
Merged

fix: emotion compatibility with latest storybook#5570
jtoar merged 6 commits intoredwoodjs:mainfrom
callingmedic911:fix/emotion-compat-with-latest-storybook-and-emotion

Conversation

@callingmedic911
Copy link
Copy Markdown
Contributor

@callingmedic911 callingmedic911 commented May 16, 2022 •
edited
Loading

This is a work in progress, I'll add more details.

Currently Storybook doesn't seem to work with the latest emotion. I had to modify some emotion fallback introduced with Chakra integration. Assuming this is related: storybookjs/storybook#16547

Need to do next:

  • Understand the context for existing withEmotionVersionFallback
  • Test compatibility with combination of older emotion (and storybook?) Removed feature flag
  • Test if Chakra is still functional
  • If checking @emotion/core is sufficient to disable emtion alias. Removed feature flag

cc: @dac09 @virtuoushub

@netlify
Copy link
Copy Markdown

netlify Bot commented May 16, 2022 •
edited
Loading

✅ Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit 1d4a2d8
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/628c5a8c58deb600089cf560

@TimKolberger
Copy link
Copy Markdown
Contributor

TimKolberger commented May 19, 2022

Hi there 👋

Understand the context for existing withEmotionVersionFallback

The function withEmotionVersionFallback handled the emotion version mismatch of storybook and Chakra UI, before the feature flag emotionAlias was added in Storybook 6.4.
It can be safely omitted, if the Storybook version is >= 6.4.

@dac09
Copy link
Copy Markdown
Contributor

dac09 commented May 19, 2022

Thanks @TimKolberger appreciate the insight!

@jtoar
Copy link
Copy Markdown
Contributor

jtoar commented May 19, 2022

Thanks @TimKolberger! We did just upgrade to Storybook 6.5—it passed CI and everything but should be tested a bit more—so we may actually be in the clear here.

@TimKolberger
Copy link
Copy Markdown
Contributor

TimKolberger commented May 19, 2022

Awesome! 🚀

@jtoar
Copy link
Copy Markdown
Contributor

jtoar commented May 20, 2022

@callingmedic911 is it possible for you try the latest canary to see if it fixes the issue?

@callingmedic911
Copy link
Copy Markdown
Contributor Author

callingmedic911 commented May 20, 2022

Yep, sure. What is changed btw?

@jtoar
Copy link
Copy Markdown
Contributor

jtoar commented May 20, 2022

@callingmedic911 the latest canary upgraded to Storybook v6.5.3 which may already fix the issue.

@callingmedic911
Copy link
Copy Markdown
Contributor Author

callingmedic911 commented May 20, 2022

@jtoar With the latest canary, I just omitted the fallback function and the storybook was up. It didn't need emotionAlias feature flag. Updating this PR.

@dac09 Pullflow.com
Copy link
Copy Markdown
Contributor

dac09 commented May 20, 2022

Remember to also double check chakra please?

@callingmedic911
Copy link
Copy Markdown
Contributor Author

callingmedic911 commented May 20, 2022

I just tried Chakra.

Ran across issue spinning up rw dev and rw storybook: Latest Chakra depends on React 18 APIs (error in @chakra-ui/hooks/dist/chakra-ui-hooks.esm.js) but Redwood doesn't support React 18 just yet.

So I downgraded to @chakra/react to v1.3.0 and the storybook was running fine with these PRs changes. So unrelated to this PR, @TimKolberger @jtoar should we pin version @chakra/react to the latest v1 in rw setup ui chakra-ui till we have React 18 support?

@callingmedic911 callingmedic911 mentioned this pull request May 20, 2022
Closed
@jtoar
Copy link
Copy Markdown
Contributor

jtoar commented May 20, 2022

@callingmedic911 yeah that's probably the best way forward. We can release a patch with that fix. Should be a one-liner right?

@jtoar jtoar mentioned this pull request May 21, 2022
Merged
@jtoar jtoar marked this pull request as ready for review May 24, 2022 04:09
jtoar
jtoar approved these changes May 24, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

This looks good. Going to get this into the minor RC, which has Storybook v6.5.4, to see if this fixes MUI for @MichaelrMentele.

@jtoar jtoar merged commit b2c2fd7 into redwoodjs:main May 24, 2022
@jtoar jtoar added this to the next-release milestone May 24, 2022
jtoar added a commit that referenced this pull request May 24, 2022
@callingmedic911 @jtoar
fix: emotion compatibility with latest storybook (#5570)
6ce2cdc 
* fix: Emotion compat with latest storybook

* Cleanup emotionAlias feature flag

* Remove unused `merge`

Co-authored-by: Dominic Saadi <[email protected]>
@jtoar jtoar modified the milestones: next-release, v1.5.0 May 24, 2022
@callingmedic911 callingmedic911 deleted the fix/emotion-compat-with-latest-storybook-and-emotion branch May 25, 2022 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jtoar jtoar jtoar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@callingmedic911 callingmedic911

Labels

release:fix This PR is a fix topic/storybook

Projects

No open projects
Main
Status: Archived

Milestone

v1.5.0

Development

Successfully merging this pull request may close these issues.

4 participants

@callingmedic911 @TimKolberger @dac09 @jtoar