fix: update next to 13.3.0 & ensure compatibility #2056
Conversation
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
github-actions
bot
added
the
type: chore
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
let's not merge this yet, i don't think the snapshot update was right as it should be failing here |
|
@orinokai How do you come to that conclusion? The |
LekoArts
changed the title
| @@ -25,7 +25,7 @@ describe('appDir', () => { | |||
| }, | |||
| followRedirect: false, | |||
| }).then((response) => { | |||
| expect(response.headers).to.have.property('content-type', 'application/octet-stream') | |||
| expect(response.headers).to.have.property('content-type', 'text/x-component') | |||
There was a problem hiding this comment.
Next.js changed their content-type so this change is expected
| @@ -31,6 +31,7 @@ const buildMiddlewareFile = async (entryPoints: Array<string>, base: string) => | |||
| format: 'esm', | |||
| target: 'esnext', | |||
| absWorkingDir: base, | |||
| external: ['next/dist/compiled/@vercel/og'], | |||
There was a problem hiding this comment.
Without this change the compilation of the middleware would fail as @vercel/og contains Node/WASM stuff
| const LOCALIZED_REGEX_PREFIX_13_1 = '(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/([^/.]{1,}))' | ||
| const OPTIONAL_REGEX_PREFIX_13_1 = '(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/([^/.]{1,}))?' | ||
|
|
||
| const LOCALIZED_REGEX_PREFIX_13_3 = '(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/((?!_next\\/)[^/.]{1,}))' |
There was a problem hiding this comment.
They changed the regex pattern for middleware matchers when using locales
| @@ -37,7 +37,7 @@ export const getResolverForDependencies = ({ | |||
| // This file is purely to allow nft to know about these pages. | |||
| exports.resolvePages = () => { | |||
| try { | |||
| ${pageFiles.join('\n ')} | |||
| ${pageFiles.sort().join('\n ')} | |||
There was a problem hiding this comment.
Needed for stable output in tests
| // it('should skip next deploy for now', () => {}) | ||
| // return | ||
| //} | ||
| if ((global as any).isNextDeploy) { |
There was a problem hiding this comment.
Next is also doing this in their tests
| @@ -2,4 +2,4 @@ export default function Page() { | |||
| return <p>pages-edge-ssr</p> | |||
| } | |||
|
|
|||
| export const config = { runtime: 'edge' } | |||
| export const runtime = 'experimental-edge' | |||
There was a problem hiding this comment.
Next is also doing this in their tests
| }) | ||
| if(!((global as any).isNextStart && process.env.CUSTOM_CACHE_HANDLER)) { | ||
| // TODO: This seems to be a bug in Next.js, try to re-enable it later | ||
| it.skip('should navigate to static path correctly', async () => { |
There was a problem hiding this comment.
This failed due to some incorrect logic in their RSC implementation. So I skipped it for now
test/e2e/app-dir/head.test.ts
Outdated
There was a problem hiding this comment.
| expect(html).toContain('<link href="/test1.js" rel="preload" as="script"/>') | ||
| expect(html).toContain('<link href="/test2.js" rel="preload" as="script"/>') | ||
| expect(html).toContain('<link href="/test3.js" rel="preload" as="script"/>') | ||
| expect(html).toContain('<link rel="preload" as="script" href="/test1.js"/>') |
There was a problem hiding this comment.
Order on things seems to have changed
| @@ -396,7 +396,7 @@ describe('app dir - rsc basics', () => { | |||
|
|
|||
| const requiredServerFiles = (await fs.readJSON(path.join(distDir, 'required-server-files.json'))).files | |||
|
|
|||
| const files = ['middleware-build-manifest.js', 'middleware-manifest.json', 'flight-manifest.json'] | |||
| const files = ['middleware-build-manifest.js', 'middleware-manifest.json', 'client-reference-manifest.json'] | |||
There was a problem hiding this comment.
They renamed this file
| @@ -105,7 +105,7 @@ describe('checkZipSize', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| describe("getProblematicUserRewrites", () => { | |||
| describeCwdTmpDir("getProblematicUserRewrites", () => { | |||
There was a problem hiding this comment.
Fix for an older PR of mine, it discovered this while running the test locally
| it('extracts dependencies that exist', async () => { | ||
| // TODO: `dependencies` references files inside the <root>/node_modules directory which isn't accessible in moveNextDist | ||
| // So this whole test needs to be reworked as it can't be fixed | ||
| it.skip('extracts dependencies that exist', async () => { |
There was a problem hiding this comment.
Because the demo page is part of the npm workspace, it references files inside the root node_modules folder.
When we copy/move this demo page to a tmp dir for this test, it tries to reach into that node_modules folder that doesn't exist.
| @@ -71,7 +71,8 @@ describe('the netlify next server', () => { | |||
| const netlifyNextServer = new NetlifyNextServer({ conf: {} }, { ...mockTokenConfig }) | |||
| const requestHandler = netlifyNextServer.getRequestHandler() | |||
|
|
|||
| const { req: mockReq, res: mockRes } = mockRequest('/getStaticProps/with-revalidate/', {}, 'GET') | |||
| const { req: mockReq, res: mockRes } = createRequestResponseMocks({ url: '/getStaticProps/with-revalidate/' }) | |||
There was a problem hiding this comment.
mockRequest was replaced by createRequestResponseMocks
Summary
Updates Next.js
nextdependency to 13.3.0Things done in this PR:
text/x-component, see Use text/x-component for RSC response vercel/next.js#45808demos/defaultchange oftsconfig.jsonandnext-end.d.tshappened automatically when running the sitenextnow seems to include@vercel/ogso theesbuildconfig had to make this part external so that Node/WASM deps are not loadedtargetin its config anymore, so we useserveras default now, if people still use Next 12 with the runtime and definetarget: serverlessit'll work. We just flipped the default. Previously this worked because Next definedserveritself as the defaultcheckIsOnDemandRevalidatereplacement ascheckIsManualRevalidatewas renamed to thismockRequestwas reworked tocreateRequestResponseMocksapp-dir/headtest as it was removed in Next: Remove head.js vercel/next.js#47507app-dir/app-edgeFixes https://github.com/netlify/pod-ecosystem-frameworks/issues/438
Fixes https://github.com/netlify/pod-ecosystem-frameworks/issues/432