-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore: 5397 Upgrade Next.js from v12 to v14 #6510
Conversation
Deployment Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6510 +/- ##
=======================================
Coverage 92.12% 92.12%
=======================================
Files 180 180
Lines 14849 14849
=======================================
Hits 13679 13679
Misses 1170 1170
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
d059e32
to
247b739
Compare
@@ -100,3 +100,5 @@ src/views/CellGuide/common/fixtures/allTissues.json | |||
src/views/CellGuide/common/fixtures/ontologyTree.json | |||
src/views/CellGuide/common/fixtures/ontologyTreeStatePerCellType.json | |||
src/views/CellGuide/common/fixtures/ontologyTreeStatePerTissue.json | |||
|
|||
certificates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nextjs built in https local FE server creates certs in this directory
"@blueprintjs/core": "^5.8.2", | ||
"@blueprintjs/icons": "^5.7.0", | ||
"@blueprintjs/select": "^5.0.23", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blueprint upgraded from v4 to v5
@@ -101,7 +100,6 @@ | |||
"eslint-plugin-sonarjs": "^0.19.0", | |||
"expect-playwright": "^0.8.0", | |||
"gray-matter": "^4.0.3", | |||
"next-dev-https": "0.2.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nexstjs now has built in https dev server mode
@@ -85,7 +85,7 @@ export const StyledCloseButtonIcon = styled(ButtonIcon, { | |||
`; | |||
|
|||
export const NewsletterModal = styled(Modal)` | |||
.bp4-dialog-header { | |||
.bp5-dialog-header { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lots of bp4
-> bp5
changes. Here and throughout
@@ -1,5 +1,5 @@ | |||
import { | |||
IMenuItemProps, | |||
MenuItemProps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blueprint js API changes
const sourceDataButton = page.getByTestId(SOURCE_DATA_BUTTON_ID); | ||
const sourceDataList = page.getByTestId(SOURCE_DATA_LIST_ID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the constants as single source of truth
await page.keyboard.press("Escape"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update test steps here, since the new version of Blueprint doesn't work with Escape reliably
"incremental": true, | ||
"plugins": [ | ||
{ | ||
"name": "next" | ||
} | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actual change from Next.js upgrade
"**/*.ts", | ||
"**/*.tsx", | ||
"**/*.js", | ||
".next/types/**/*.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actual change from Next.js upgrade
@@ -5,24 +5,48 @@ | |||
"esModuleInterop": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lots of prettier changes
970aa5d
to
0275cad
Compare
@@ -24,7 +25,7 @@ const AsyncCTA = loadable( | |||
/*webpackChunkName: 'CreateCollectionModalCTA' */ import("./components/CTA") | |||
); | |||
|
|||
const CreateCollectionButton = (props: Partial<IButtonProps>) => ( | |||
const CreateCollectionButton = (props: Partial<ButtonProps>) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice pick up thanks @tihuan! ⭐
popoverProps?: IPopoverProps; | ||
buttonProps?: IButtonProps; | ||
popoverProps?: PopoverProps; | ||
buttonProps?: Partial<ButtonIconProps<"dotsHorizontal", "small">>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tihuan thank you 🎆
Hi @tihuan, I'm seeing a new warnings in my terminal. Are you seeing these too?
And |
Thanks so much for the thorough testing, @frano-m ! Sorry that I missed those warnings 🙏 I'll fix them today! I can repro the NTag warning too 🙆♂️ |
68108ef
to
5ce5bfe
Compare
@frano-m I fixed the layout effect error (caused by using Blueprint AnchorButton The |
@@ -24,58 +23,42 @@ export default function Nav({ className, pathname }: Props): JSX.Element { | |||
<NavSection> | |||
<NavSectionTitle>Application</NavSectionTitle> | |||
<NavItemContainer> | |||
<LinkWrapper> | |||
<Link href={ROUTES.COLLECTIONS} passHref> | |||
<AnchorButton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seve I removed AnchorButton
usage since it violates React 18 usage. I think the nav link still works after I updated the style. But PTAL in case I missed anything 🙏 Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @tihuan! Nice work and thank you 🚀
Amazing! Thanks so much again for catching the errors, @frano-m 🤩 🙏 ! |
Reason for Change
Changes
next/image
andnext/link
next-dev-https
, since Next.js now has their own dev server https modeKnown Issues
Testing steps
Checklist 🛎️
Notes for Reviewer