-
Notifications
You must be signed in to change notification settings - Fork 87
fix: don't move files to the CDN if they match redirect/rewrite rules #832
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
Merged
Merged
Changes from 2 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
const Images = () => ( | ||
<div> | ||
<p>If you can see this, rewrites aren't working</p> | ||
</div> | ||
) | ||
|
||
export default Images |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
const Redirect = () => ( | ||
<div> | ||
<p>If you can see this, redirects aren't working</p> | ||
</div> | ||
) | ||
|
||
export default Redirect |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
// @ts-check | ||
/* eslint-disable max-lines */ | ||
const { cpus } = require('os') | ||
|
||
const { yellowBright } = require('chalk') | ||
|
@@ -28,10 +28,36 @@ const matchMiddleware = (middleware, filePath) => | |
filePath === middlewarePath || filePath === `${middlewarePath}.html` || filePath.startsWith(`${middlewarePath}/`), | ||
) | ||
|
||
const matchesRedirect = (file, redirects) => { | ||
if (!Array.isArray(redirects)) { | ||
return false | ||
} | ||
return redirects.some((redirect) => { | ||
if (!redirect.regex || redirect.internal) { | ||
return false | ||
} | ||
// Strips the extension from the file path | ||
return new RegExp(redirect.regex).test(`/${file.slice(0, -5)}`) | ||
}) | ||
} | ||
|
||
const matchesRewrite = (file, rewrites) => { | ||
if (Array.isArray(rewrites)) { | ||
return matchesRedirect(file, rewrites) | ||
} | ||
if (!Array.isArray(rewrites?.beforeFiles)) { | ||
return false | ||
} | ||
return matchesRedirect(file, rewrites.beforeFiles) | ||
} | ||
|
||
exports.matchesRedirect = matchesRedirect | ||
exports.matchesRewrite = matchesRewrite | ||
exports.matchMiddleware = matchMiddleware | ||
exports.stripLocale = stripLocale | ||
exports.isDynamicRoute = isDynamicRoute | ||
|
||
// eslint-disable-next-line max-lines-per-function | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [sand] Same for eslint |
||
exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => { | ||
console.log('Moving static page files to serve from CDN...') | ||
const outputDir = join(netlifyConfig.build.publish, target === 'server' ? 'server' : 'serverless') | ||
|
@@ -48,6 +74,7 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => { | |
} | ||
|
||
const prerenderManifest = await readJson(join(netlifyConfig.build.publish, 'prerender-manifest.json')) | ||
const { redirects, rewrites } = await readJson(join(netlifyConfig.build.publish, 'routes-manifest.json')) | ||
|
||
const isrFiles = new Set() | ||
|
||
|
@@ -79,6 +106,8 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => { | |
|
||
const matchingMiddleware = new Set() | ||
const matchedPages = new Set() | ||
const matchedRedirects = new Set() | ||
const matchedRewrites = new Set() | ||
|
||
// Limit concurrent file moves to number of cpus or 2 if there is only 1 | ||
const limit = pLimit(Math.max(2, cpus().length)) | ||
|
@@ -91,6 +120,14 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => { | |
if (isDynamicRoute(filePath)) { | ||
return | ||
} | ||
if (matchesRedirect(filePath, redirects)) { | ||
matchedRedirects.add(filePath) | ||
return | ||
} | ||
if (matchesRewrite(filePath, rewrites)) { | ||
matchedRewrites.add(filePath) | ||
return | ||
} | ||
// Middleware matches against the unlocalised path | ||
const unlocalizedPath = stripLocale(rawPath, i18n?.locales) | ||
const middlewarePath = matchMiddleware(middleware, unlocalizedPath) | ||
|
@@ -110,7 +147,8 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => { | |
yellowBright(outdent` | ||
Skipped moving ${matchedPages.size} ${ | ||
matchedPages.size === 1 ? 'file because it matches' : 'files because they match' | ||
} middleware, so cannot be deployed to the CDN and will be served from the origin instead. This is fine, but we're letting you know because it may not be what you expect. | ||
} middleware, so cannot be deployed to the CDN and will be served from the origin instead. | ||
This is fine, but we're letting you know because it may not be what you expect. | ||
`), | ||
) | ||
|
||
|
@@ -119,6 +157,8 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => { | |
The following middleware matched statically-rendered pages: | ||
|
||
${yellowBright([...matchingMiddleware].map((mid) => `- /${mid}/_middleware`).join('\n'))} | ||
|
||
──────────────────────────────────────────────────────────────── | ||
`, | ||
) | ||
// There could potentially be thousands of matching pages, so we don't want to spam the console with this | ||
|
@@ -128,6 +168,40 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => { | |
The following files matched middleware and were not moved to the CDN: | ||
|
||
${yellowBright([...matchedPages].map((mid) => `- ${mid}`).join('\n'))} | ||
|
||
──────────────────────────────────────────────────────────────── | ||
`, | ||
) | ||
} | ||
} | ||
|
||
if (matchedRedirects.size !== 0 || matchedRewrites.size !== 0) { | ||
console.log( | ||
yellowBright(outdent` | ||
Skipped moving ${ | ||
matchedRedirects.size + matchedRewrites.size | ||
} files because they match redirects or beforeFiles rewrites, so cannot be deployed to the CDN and will be served from the origin instead. | ||
`), | ||
) | ||
if (matchedRedirects.size < 50 && matchedRedirects.size !== 0) { | ||
console.log( | ||
outdent` | ||
The following files matched redirects and were not moved to the CDN: | ||
|
||
${yellowBright([...matchedRedirects].map((mid) => `- ${mid}`).join('\n'))} | ||
|
||
──────────────────────────────────────────────────────────────── | ||
`, | ||
) | ||
} | ||
if (matchedRewrites.size < 50 && matchedRewrites.size !== 0) { | ||
console.log( | ||
outdent` | ||
The following files matched beforeFiles rewrites and were not moved to the CDN: | ||
|
||
${yellowBright([...matchedRewrites].map((mid) => `- ${mid}`).join('\n'))} | ||
|
||
──────────────────────────────────────────────────────────────── | ||
`, | ||
) | ||
} | ||
|
@@ -151,3 +225,4 @@ exports.movePublicFiles = async ({ appDir, publish }) => { | |
await copy(publicDir, `${publish}/`) | ||
} | ||
} | ||
/* eslint-enable max-lines */ |
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
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.
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.
[sand] We should probably avoid disabling eslint and separate this file into smaller ones moving forward