fix: publish from subdirectory #1756
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ 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-export-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-canary 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. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-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-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
ascorbic
force-pushed
the
mk/publish-dir
branch
from
6b83c9a to
2558855
Compare
ascorbic
commented
Nov 10, 2022
| @@ -25,14 +25,6 @@ import { | |||
| const matchesMiddleware = (middleware: Array<string>, route: string): boolean => | |||
| middleware.some((middlewarePath) => route.startsWith(middlewarePath)) | |||
|
|
|||
| const generateHiddenPathRedirects = ({ basePath }: Pick<NextConfig, 'basePath'>): NetlifyConfig['redirects'] => | |||
| HIDDEN_PATHS.map((path) => ({ | |||
There was a problem hiding this comment.
Not needed anymore, as these aren't published
There was a problem hiding this comment.
They were the files in .next that we didn't want to be publicly-visible in the deploy, such as cache, server and various json files
ascorbic
commented
Nov 10, 2022
| @@ -66,21 +58,6 @@ const generateLocaleRedirects = ({ | |||
| return redirects | |||
| } | |||
|
|
|||
| export const generateStaticRedirects = ({ | |||
| netlifyConfig, | |||
There was a problem hiding this comment.
No need to redirect, because the files are in the right place
ascorbic
commented
Nov 10, 2022
|
|
||
| if (i18n) { | ||
| netlifyConfig.redirects.push({ from: `${basePath}/:locale/_next/static/*`, to: `/static/:splat`, status: 200 }) | ||
| } |
There was a problem hiding this comment.
Seems i18n still loads from /_next in the root, so it's not needed
sarahetter
reviewed
Nov 10, 2022
| }): Promise<void> => { | ||
| console.log('Moving static page files to serve from CDN...') | ||
| const outputDir = join(netlifyConfig.build.publish, target === 'server' ? 'server' : 'serverless') | ||
| const outputDir = join(distDir, 'server') |
There was a problem hiding this comment.
Why have we removed serverless from here?
There was a problem hiding this comment.
We force builds to use server target anyway, and in Next 13 serverless breaks the build.
Merged
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Currently we expect Next.js to publish the
distDir(i.e..next). This is how we ascertain how to find the build files, particularlyrequired-server-files.json. However most of the generated outputs are not created in the correct structure, so we need to copy or move them there as part of the build. This has worked fine so far, but has some important drawbacks. The main one is that it means the whole folder is uploaded as part of the deploy, even though most of it is not needed - and in fact needs 404 redirects to avoid making them publicly available. This hasn't been a major problem in the past (once the first deploy has been uploaded there aren't many extra files that need uploading). However Next 13 has started generating very large cache files that change on every build, which are now being uploaded every time.Since this plugin was created it is now possible to change the publish dir from within a build plugin. This gives us another option that retains the best of these worlds. We can use
.nextas the default publish dir, but during the build change it to a hidden.next/distdir, which then contains the actual files.Test plan
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.