Skip to content

Commit 5d65c92

Browse files
pieheduardoboucas
andauthored
fix: adjust bundling to not produce duplicate inlined internal modules (#2774)
* chore: add some validation to bundling to ensure we don't produce duplicate/inling of regional-blob-store module in unexptected built modules * chore: don't hardcode repo directory in build script * test: adjust assertion for case where multiple cache-status values are being comma separated and not line separated * Update tools/build.js Co-authored-by: Eduardo Bouças <[email protected]> --------- Co-authored-by: Eduardo Bouças <[email protected]>
1 parent 23b1f37 commit 5d65c92

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

tests/e2e/simple-app.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ test('Renders the Home page correctly', async ({ page, simple }) => {
1212

1313
await expect(page).toHaveTitle('Simple Next App')
1414

15-
expect(headers['cache-status']).toMatch(/^"Next.js"; hit$/m)
16-
expect(headers['cache-status']).toMatch(/^"Netlify Edge"; fwd=miss$/m)
15+
expect(headers['cache-status'].replaceAll(', ', '\n')).toMatch(/^"Next.js"; hit$/m)
16+
expect(headers['cache-status'].replaceAll(', ', '\n')).toMatch(/^"Netlify Edge"; fwd=miss$/m)
1717
// "Netlify Durable" assertion is skipped because we are asserting index page and there are possible that something else is making similar request to it
1818
// and as a result we can see many possible statuses for it: `fwd=miss`, `fwd=miss; stored`, `hit; ttl=<ttl>` so there is no point in asserting on that
1919
// "Netlify Edge" status suffers from similar issue, but is less likely to manifest (only if those requests would be handled by same CDN node) and retries

tools/build.js

+36-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createWriteStream } from 'node:fs'
2-
import { cp, rm } from 'node:fs/promises'
3-
import { join, resolve } from 'node:path'
2+
import { cp, readFile, rm } from 'node:fs/promises'
3+
import { dirname, join, resolve } from 'node:path'
4+
import { fileURLToPath } from 'node:url'
45
import { Readable } from 'stream'
56
import { finished } from 'stream/promises'
67

@@ -11,6 +12,8 @@ import glob from 'fast-glob'
1112
const OUT_DIR = 'dist'
1213
await rm(OUT_DIR, { force: true, recursive: true })
1314

15+
const repoDirectory = dirname(resolve(fileURLToPath(import.meta.url), '..'))
16+
1417
const entryPointsESM = await glob('src/**/*.ts', { ignore: ['**/*.test.ts'] })
1518
const entryPointsCJS = await glob('src/**/*.cts')
1619

@@ -39,7 +42,7 @@ async function bundle(entryPoints, format, watch) {
3942
name: 'mark-runtime-modules-as-external',
4043
setup(pluginBuild) {
4144
pluginBuild.onResolve({ filter: /^\..*\.c?js$/ }, (args) => {
42-
if (args.importer.includes(join('opennextjs-netlify', 'src'))) {
45+
if (args.importer.includes(join(repoDirectory, 'src'))) {
4346
return { path: args.path, external: true }
4447
}
4548
})
@@ -126,8 +129,38 @@ await Promise.all([
126129
cp('src/build/templates', join(OUT_DIR, 'build/templates'), { recursive: true, force: true }),
127130
])
128131

132+
async function ensureNoRegionalBlobsModuleDuplicates() {
133+
const REGIONAL_BLOB_STORE_CONTENT_TO_FIND = 'fetchBeforeNextPatchedIt'
134+
135+
const filesToTest = await glob(`${OUT_DIR}/**/*.{js,cjs}`)
136+
const unexpectedModulesContainingFetchBeforeNextPatchedIt = []
137+
let foundInExpectedModule = false
138+
for (const fileToTest of filesToTest) {
139+
const content = await readFile(fileToTest, 'utf-8')
140+
if (content.includes(REGIONAL_BLOB_STORE_CONTENT_TO_FIND)) {
141+
if (fileToTest.endsWith('run/regional-blob-store.cjs')) {
142+
foundInExpectedModule = true
143+
} else {
144+
unexpectedModulesContainingFetchBeforeNextPatchedIt.push(fileToTest)
145+
}
146+
}
147+
}
148+
if (!foundInExpectedModule) {
149+
throw new Error(
150+
'Expected to find "fetchBeforeNextPatchedIt" variable in "run/regional-blob-store.cjs", but it was not found. This might indicate a setup change that requires the bundling validation in "tools/build.js" to be adjusted.',
151+
)
152+
}
153+
if (unexpectedModulesContainingFetchBeforeNextPatchedIt.length !== 0) {
154+
throw new Error(
155+
`Bundling produced unexpected duplicates of "regional-blob-store" module in following built modules:\n${unexpectedModulesContainingFetchBeforeNextPatchedIt.map((filePath) => ` - ${filePath}`).join('\n')}`,
156+
)
157+
}
158+
}
159+
129160
if (watch) {
130161
console.log('Starting compilation in watch mode...')
131162
} else {
163+
await ensureNoRegionalBlobsModuleDuplicates()
164+
132165
console.log('Finished building 🎉')
133166
}

0 commit comments

Comments
 (0)