Skip to content

Conversation

rohit-nair
Copy link
Contributor

@rohit-nair rohit-nair commented May 7, 2022

2/

[Note: This is a stacked PR on top of #370 . Can't set the base branch to rohit-nair:rn/react-router-upgrade here, most likely as I'm working off of a fork. There are only 2 commits as part of this PR — 809de8b, 2cddde3.]

Upgrading storybook to v6.x to address issue #360 .
Got storybook site to load correctly post upgrade.

  • Ran npx sb upgrade to upgrade storybook from v5.3 to v6.4.
  • Fixed glob pattern as it was incorrect.
  • Disable feature flag to opt-out of the previous behavior of pinning the Emotion version to v10.
  • Fix Uncaught TypeError: Cannot read properties of undefined (reading 'Pop') error. My suspicion is that its due to incompatibility with history@4.10.1. May require upgrading react-router as well. Fixed by rebasing off of deps: Upgrade react-router to v6 #370 .

image

Attached is the log from the upgrade.

@rohit-nair rohit-nair force-pushed the rn/storybook-upgrade-v6 branch from 8e0f792 to 2cddde3 Compare May 29, 2022 07:38
@rohit-nair rohit-nair marked this pull request as ready for review May 29, 2022 07:47
@rohit-nair rohit-nair force-pushed the rn/storybook-upgrade-v6 branch from 2cddde3 to 55e17d8 Compare June 1, 2022 00:33
Copy link
Member

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

tACK. Code looks good. What a whirlwind just to get storybook back up and running. Thanks again for tackling this issue.

This will just need a rebase after #370 is merged.

@jamaljsr
Copy link
Member

jamaljsr commented Jun 2, 2022

Now that #370 is merged, can you rebase this one?

Ran `npx sb upgrade` to upgrade storybook from v5.3 to v6.5.
Answer `Yes` to use webpack5 manager.
Disable feature flag to opt-out of the previous behavior of
pinning the Emotion version to v10.
@rohit-nair rohit-nair force-pushed the rn/storybook-upgrade-v6 branch from 55e17d8 to e2a79ca Compare June 2, 2022 16:38
@rohit-nair
Copy link
Contributor Author

Rebased this PR.
I'll keep an eye out for issues. If you see or hear something, feel free tag/ping me.

Copy link
Member

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks again for your contribution to the project.

@jamaljsr jamaljsr merged commit 6630f46 into lightninglabs:master Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants