fix: adjust bundling to not produce duplicate inlined internal modules #2774
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
pieh
force-pushed
the
michalpiechowiak/frb-1681-fix-next-runtime-bundling-so-we-dont-duplicateinline
branch
3 times, most recently
from
1b57a7b to
2397c8a
Compare
📊 Package size report No changes
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
pieh
commented
Mar 17, 2025
tools/build.js
Outdated
Comment on lines
153
to
157
| if (unexpectedModulesContainingFetchBeforeNextPatchedIt.length !== 0) { | ||
| throw new Error( | ||
| `Bundling produced unexpected duplicates of 'regional-blob-store' module in following built modules:\n${unexpectedModulesContainingFetchBeforeNextPatchedIt.map((filePath) => ` - ${filePath}`).join('\n')}`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
This produce error like this (when using just validation part of this PR, without the fix part):
Error: Bundling produced unexpected duplicates of 'regional-blob-store' module in following built modules:
- dist/esm-chunks/chunk-K4RDUZYO.js
- dist/run/next.cjs
- dist/run/handlers/cache.cjs
at ensureNoRegionalBlobsModuleDuplicates (file:///Users/misiek/dev/pgs-next-runtime/tools/build.js:154:11)
at async file:///Users/misiek/dev/pgs-next-runtime/tools/build.js:163:3
The main purpose is to prevent publishing mis-bundlend version of runtime from local machines.
With the fix validation is passing (as avoiding duplicates is working then)
pieh
commented
Mar 17, 2025
| @@ -39,7 +42,7 @@ async function bundle(entryPoints, format, watch) { | |||
| name: 'mark-runtime-modules-as-external', | |||
| setup(pluginBuild) { | |||
| pluginBuild.onResolve({ filter: /^\..*\.c?js$/ }, (args) => { | |||
| if (args.importer.includes(join('opennextjs-netlify', 'src'))) { | |||
| if (args.importer.includes(join(repoDirectory, 'src'))) { | |||
There was a problem hiding this comment.
Main fix/adjustment - we were publishing runtime locally since move to Opennext organization and if locally repo clone directory was not named exactly opennextjs-neltify, then we would produce duplicates.
pieh
force-pushed
the
michalpiechowiak/frb-1681-fix-next-runtime-bundling-so-we-dont-duplicateinline
branch
from
796ba09 to
14ce8e3
Compare
…licate/inling of regional-blob-store module in unexptected built modules
pieh
force-pushed
the
michalpiechowiak/frb-1681-fix-next-runtime-bundling-so-we-dont-duplicateinline
branch
from
ad1ad5d to
3fbf588
Compare
…e being comma separated and not line separated
pieh
force-pushed
the
michalpiechowiak/frb-1681-fix-next-runtime-bundling-so-we-dont-duplicateinline
branch
from
52811fa to
14f2b98
Compare
pieh
commented
Mar 17, 2025
Comment on lines
+15
to
+16
| expect(headers['cache-status'].replaceAll(', ', '\n')).toMatch(/^"Next.js"; hit$/m) | ||
| expect(headers['cache-status'].replaceAll(', ', '\n')).toMatch(/^"Netlify Edge"; fwd=miss$/m) |
There was a problem hiding this comment.
Not related to main portion of PR - seems like we might be getting comma separated cache-status values, so I'm massing it to have each statement as separate line to match regex assumption
eduardoboucas
previously approved these changes
Mar 17, 2025
Co-authored-by: Eduardo Bouças <[email protected]>
eduardoboucas
approved these changes
Mar 17, 2025
pieh
changed the title
pieh
deleted the
michalpiechowiak/frb-1681-fix-next-runtime-bundling-so-we-dont-duplicateinline
branch
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.
Description
We are producing duplicate inlining in built package if built in directory that is not named
opennextjs-netlify. This adjusts build script to not rely on directory name and adds some validation.This is still a bit bandaidy - ideally we don't do multiple passes and do single build emitting mix of ESM and CJS, but I couldn't find to make
esbuildbehave in this way, so to not overhaul entire setup and spend a lot of time on it - I just made some adjustments and really barebones verification/validation of built package.Tests
Adding some validation to build script
Relevant links (GitHub issues, etc.) or a picture of cute animal
https://linear.app/netlify/issue/FRB-1681/fix-next-runtime-bundling-so-we-dont-duplicateinline-internal-modules