Skip to content

fix: correct handling of data route JSON files #864

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.

Sign up for GitHub

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 5 commits into from
Dec 2, 2021
Merged

fix: correct handling of data route JSON files #864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62c24a7
chore: update demo page links
ascorbic Dec 2, 2021
e1d6c4e
fix: correct handling of CDN files
ascorbic Dec 2, 2021
663f8c7
chore: fix tests
ascorbic Dec 2, 2021
7543385
Merge branch 'main' into mk/static-props-test
ascorbic Dec 2, 2021
7623cc6
Merge branch 'main' into mk/static-props-test
kodiakhq[bot] Dec 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions demos/default/next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ module.exports = {
destination: '/:path*',
},
],
afterFiles: [{
source: '/rewriteToStatic',
destination: '/getStaticProps/1',
}]
}
},
// Redirects allow you to redirect an incoming request path to a different destination path.
Expand Down
75 changes: 74 additions & 1 deletion demos/default/pages/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const Index = ({ shows }) => {
))}
</ul>

<h2>Catch-All Routess</h2>
<h2>Catch-All Routes</h2>

<ul data-testid="list-catch-all">
<li>
Expand Down Expand Up @@ -105,6 +105,79 @@ const Index = ({ shows }) => {
</Link>
</li>
</ul>
<h2>Page types</h2>
<ul>
<li>
<Link href="/getServerSideProps/1">
<a>/getServerSideProps/1</a>
</Link>
</li>
<li>
<Link href="/getServerSideProps/static">
<a>/getServerSideProps/static</a>
</Link>
</li>
<li>
<Link href="/getServerSideProps/all/1">
<a>/getServerSideProps/all/1</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/1">
<a>/getStaticProps/1</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/static">
<a>/getStaticProps/static</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/with-revalidate">
<a>/getStaticProps/with-revalidate</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/withFallback/3">
<a>/getStaticProps/withFallback/3 (pre-rendered)</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/withFallback/300">
<a>/getStaticProps/withFallback/300 (SSR)</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/withFallbackBlocking/300">
<a>/getStaticProps/withFallbackBlocking/300 (SSR)</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/withRevalidate/2">
<a>/getStaticProps/withRevalidate/2</a>
</Link>
</li>
<li>
<Link href="/getStaticProps/withRevalidate/withFallback/200">
<a>/getStaticProps/withRevalidate/withFallback/200</a>
</Link>
</li>
<li>
<Link href="/old/image">
<a>Rewrite (should display image)</a>
</Link>
</li>
<li>
<Link href="/rewriteToStatic">
<a>Rewrite to static (should show getStaticProps/1)</a>
</Link>
</li>
<li>
<Link href="/middle">
<a>Middleware</a>
</Link>
</li>
</ul>
</div>
)
}
Expand Down
27 changes: 23 additions & 4 deletions src/helpers/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@
const { cpus } = require('os')

const { yellowBright } = require('chalk')
const { existsSync, readJson, move, cpSync, copy, writeJson, readFile, writeFile } = require('fs-extra')
const {
existsSync,
readJson,
move,
copy,
writeJson,
readFile,
writeFile,
ensureDir,
readFileSync,
} = require('fs-extra')
const globby = require('globby')
const { outdent } = require('outdent')
const pLimit = require('p-limit')
Expand Down Expand Up @@ -62,7 +72,9 @@ 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')
const root = join(outputDir, 'pages')

const buildId = readFileSync(join(netlifyConfig.build.publish, 'BUILD_ID'), 'utf8').trim()
const dataDir = join('_next', 'data', buildId)
await ensureDir(dataDir)
// Load the middleware manifest so we can check if a file matches it before moving
let middleware
const manifestPath = join(outputDir, 'middleware-manifest.json')
Expand All @@ -88,10 +100,17 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
})

const files = []
const filesManifest = {}
const moveFile = async (file) => {
const isData = file.endsWith('.json')
const source = join(root, file)
const target = isData ? join(dataDir, file) : file

files.push(file)
const dest = join(netlifyConfig.build.publish, file)
filesManifest[file] = target

const dest = join(netlifyConfig.build.publish, target)

try {
await move(source, dest)
} catch (error) {
Expand Down Expand Up @@ -208,7 +227,7 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
}

// Write the manifest for use in the serverless functions
await writeJson(join(netlifyConfig.build.publish, 'static-manifest.json'), files)
await writeJson(join(netlifyConfig.build.publish, 'static-manifest.json'), Object.entries(filesManifest))

if (i18n?.defaultLocale) {
// Copy the default locale into the root
Expand Down
39 changes: 33 additions & 6 deletions src/templates/getHandler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable max-lines-per-function */
const { promises, existsSync } = require('fs')
const { Server } = require('http')
const { tmpdir } = require('os')
Expand Down Expand Up @@ -34,12 +35,14 @@ const makeHandler =
// In most cases these are served from the CDN, but for rewrites Next may try to read them
// from disk. We need to intercept these and load them from the CDN instead
// Sadly the only way to do this is to monkey-patch fs.promises. Yeah, I know.
const staticFiles = new Set(staticManifest)

const staticFiles = new Map(staticManifest)
const downloadPromises = new Map()
const statsCache = new Map()
// Yes, you can cache stuff locally in a Lambda
const cacheDir = path.join(tmpdir(), 'next-static-cache')
// Grab the real fs.promises.readFile...
const readfileOrig = promises.readFile
const statsOrig = promises.stat
// ...then money-patch it to see if it's requesting a CDN file
promises.readFile = async (file, options) => {
// We only care about page files
Expand All @@ -51,13 +54,24 @@ const makeHandler =
if (staticFiles.has(filePath) && !existsSync(file)) {
// This name is safe to use, because it's one that was already created by Next
const cacheFile = path.join(cacheDir, filePath)
// Have we already cached it? We ignore the cache if running locally to avoid staleness
const url = `${base}/${staticFiles.get(filePath)}`

// If it's already downloading we can wait for it to finish
if (downloadPromises.has(url)) {
await downloadPromises.get(url)
}
// Have we already cached it? We download every time if running locally to avoid staleness
if ((!existsSync(cacheFile) || process.env.NETLIFY_DEV) && base) {
await promises.mkdir(path.dirname(cacheFile), { recursive: true })

// Append the path to our host and we can load it like a regular page
const url = `${base}/${filePath}`
await downloadFile(url, cacheFile)
try {
// Append the path to our host and we can load it like a regular page
const downloadPromise = downloadFile(url, cacheFile)
downloadPromises.set(url, downloadPromise)
await downloadPromise
} finally {
downloadPromises.delete(url)
}
}
// Return the cache file
return readfileOrig(cacheFile, options)
Expand All @@ -66,6 +80,18 @@ const makeHandler =

return readfileOrig(file, options)
}

promises.stat = async (file, options) => {
// We only care about page files
if (file.startsWith(pageRoot)) {
// We only want the part after `pages/`
const cacheFile = path.join(cacheDir, file.slice(pageRoot.length + 1))
if (existsSync(cacheFile)) {
return statsOrig(cacheFile, options)
}
}
return statsOrig(file, options)
}
}
let NextServer
try {
Expand Down Expand Up @@ -183,3 +209,4 @@ exports.handler = ${
`

module.exports = getHandler
/* eslint-enable max-lines-per-function */
Loading