fix: support appDir #1638
fix: support appDir #1638
Conversation
✅ 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 netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-hp-edge-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-rsc-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 netlify-plugin-nextjs-export-demo 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 netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
This comment was marked as outdated.
This comment was marked as outdated.
✅ 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. |
ascorbic
force-pushed
the
mk/canary-demo
branch
4 times, most recently
from
e6ef84b to
baacc4d
Compare
ascorbic
force-pushed
the
mk/canary-demo
branch
from
baacc4d to
2c85686
Compare
There was a problem hiding this comment.
Looks great, @ascorbic! I did a really thorough test today (partly as a learning exercise) of server/client components, static/dynamic rendering, static/dynamic paths and various revalidate/fallback options. All looks great and the tests do what they should. 🚀
ascorbic
force-pushed
the
mk/canary-demo
branch
from
b03e35a to
1c3cc3e
Compare
ascorbic
force-pushed
the
mk/canary-demo
branch
from
126fd93 to
4e89a33
Compare
| const { host } = event.headers | ||
| const protocol = event.headers['x-forwarded-proto'] || 'http' | ||
| base = `${protocol}://${host}` | ||
| base = url.origin |
There was a problem hiding this comment.
origin is protocol://host:port
| if (event.headers['x-middleware-prefetch'] && mode === 'ssr') { | ||
| return { | ||
| statusCode: 200, | ||
| body: '{}', |
There was a problem hiding this comment.
Yes, this is the correct value. https://github.com/vercel/next.js/blob/canary/packages/next/server/base-server.ts#L1068-L1072
| @@ -201,6 +201,8 @@ export const getPrefetchResponse = (event: HandlerEvent, mode: string): HandlerR | |||
| headers: { | |||
| 'Content-Type': 'application/json', | |||
| 'x-middleware-skip': '1', | |||
| // https://github.com/vercel/next.js/pull/42936/files#r1027563953 | |||
| vary: 'x-middleware-prefetch', | |||
There was a problem hiding this comment.
Good catch, what prompted the need for this?
There was a problem hiding this comment.
I'd not hit the problem, but saw that user's comment when investigating the feature. I could see it being an issue at some point
| @@ -10,10 +10,10 @@ describe('Default site', () => { | |||
| cy.url().should('eq', `${Cypress.config().baseUrl}/`) | |||
| }) | |||
|
|
|||
| it('serves generated public files', async () => { | |||
| cy.request('service-worker.js').then((res) => { | |||
| it('serves generated public files', () => { | |||
There was a problem hiding this comment.
Is this test change related to the appDir work? Just trying to understand the connection between appDir work and removing a service worker call from the Nx demo project.
There was a problem hiding this comment.
It was failing tests
Summary
Add support for Next 13 app dir. This should have no effect unless a user has enabled
experimental.appDirin thenext.config.js. Adds a test suite that has been ported from the e2e tests in the Next.js monorepo.Known issues
The following are known issues:
dynamicParamsReviewing
Nothing in
testneeds review, so fear not: there are only about 8 actual source files that need reviewing.Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.