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

upgrade to react 18, storybook -> ladle #279

Merged
merged 27 commits into from
Nov 10, 2022
Merged

upgrade to react 18, storybook -> ladle #279

merged 27 commits into from
Nov 10, 2022

Conversation

davidjgoss
Copy link
Contributor

@davidjgoss davidjgoss commented Aug 26, 2022

🤔 What's changed?

  • Upgrade to React 18, change code and dependencies appropriately
  • Remove Storybook and use Ladle instead, since the former seemed to struggle with React 18 and we've been wanting to move away from it for a while anyway

⚡️ What's your motivation?

We want to keep current with React's major version.

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)

♻️ Anything particular you want feedback on?

I've had to temporarily disable the build of the Ladle app from the test workflow because the production build (it's Vite and Rollup under the hood) errors on elasticlunr, which has some invalid code that doesn't pass strict mode, and is also now unmaintained. The original lunr.js has both the same issues too. I'm exploring switching to https://github.com/LyraSearch/lyra and will raise a follow-up PR with either that or a hack to work around the elasticlunr issue.

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@davidjgoss davidjgoss changed the title (wip) react 18 (wip) react 18, ladle Aug 27, 2022
@davidjgoss davidjgoss changed the title (wip) react 18, ladle upgrade to react 18, storybook -> ladle Nov 6, 2022
@davidjgoss davidjgoss marked this pull request as ready for review November 9, 2022 14:16
@mpkorstanje
Copy link
Contributor

I've had to temporarily disable the build of the Ladle app from the test workflow because the production build

Does this imply that after merging the project is not releasable?

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Previous remark aside, this looks like a fairly standaard upgrade to me.

@davidjgoss
Copy link
Contributor Author

Does this imply that after merging the project is not releasable?

No - sorry, I wasn't very clear there. It stops the production bundle working on the Ladle (nee Storybook) project, but we weren't doing anything with that yet anyway. The npm package we release and use in the HTML formatter is unaffected.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Nov 10, 2022

I believe Matt and Aslak are on holidays, more or less, this week.

So you may want to merge now before Renovate gets you again.

@davidjgoss davidjgoss merged commit 2866212 into main Nov 10, 2022
@davidjgoss davidjgoss deleted the react-18 branch November 10, 2022 11:57
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