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 Next.js 12, EUI 41.3 #47

Merged
merged 7 commits into from
Nov 29, 2021

Conversation

pugnascotia
Copy link
Collaborator

@pugnascotia pugnascotia commented Nov 29, 2021

This PR updates all dependencies apart from EUI. Notably, that means an upgrade of Next.js from v11 to v12. There were surprisingly few changes to make this work. ESLint had to be kept back because Next.js complained about using the latest version. I expect that to be relaxed in the future.

Future PRs:

  • Upgrade EUI Actually, upgrading EUI turned out to be trivial so I've included it.
  • Try using Next's build-in ESLint support in order to reduce config in the starter

@pugnascotia pugnascotia added enhancement New feature or request dependencies Pull requests that update a dependency file labels Nov 29, 2021
@pugnascotia pugnascotia changed the title Upgrade to Next.js 12 Upgrade to Next.js 12, EUI 41.3 Nov 29, 2021
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Code changes LGTM. Spot checked the app locally and it all looks good except this console error after yarn dev:

Screen Shot 2021-11-29 at 11 14 23 AM

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Thanks again Rory! Changes LGTM, tested with both dev & prod modes.

I pushed a complete regeneration of yarn.lock to opt into some additional updates & clean up duplicated packages.

@chandlerprall
Copy link
Contributor

Code changes LGTM. Spot checked the app locally and it all looks good except this console error after yarn dev:

Screen Shot 2021-11-29 at 11 14 23 AM

This comes from EuiSideNav and is an unconfigurable thing, https://github.com/elastic/eui/blob/main/src/components/side_nav/side_nav.tsx#L188

Thankfully, apart from this warning, nothing breaks (unless you're on a really old react@15 version) and React updates the id + aria-controls appropriately. I've opened elastic/eui#5418 to track this, and an SSR meta issue in EUI as well elastic/eui#5419

@pugnascotia pugnascotia merged commit ce7f5bf into elastic:master Nov 29, 2021
@pugnascotia pugnascotia deleted the upgrade-nextjs-12 branch November 29, 2021 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants