feat(nextjs): Update default ignore list for sourcemaps#18938
Conversation
…could-not-determine-source-map-reference
| MAIN_CHUNKS: '**/main-*', | ||
| FRAMEWORK_CHUNKS: '**/framework-*', | ||
| FRAMEWORK_CHUNKS_DOT: '**/framework.*', | ||
| POLYFILLS_CHUNKS: '**/polyfills-*', | ||
| WEBPACK_CHUNKS: '**/webpack-*', | ||
| PAGE_CLIENT_REFERENCE_MANIFEST: '**/page_client-reference-manifest.js', | ||
| SERVER_REFERENCE_MANIFEST: '**/server-reference-manifest.js', | ||
| NEXT_FONT_MANIFEST: '**/next-font-manifest.js', | ||
| MIDDLEWARE_BUILD_MANIFEST: '**/middleware-build-manifest.js', | ||
| INTERCEPTION_ROUTE_REWRITE_MANIFEST: '**/interception-route-rewrite-manifest.js', | ||
| ROUTE_CLIENT_REFERENCE_MANIFEST: '**/route_client-reference-manifest.js', | ||
| MIDDLEWARE_REACT_LOADABLE_MANIFEST: '**/middleware-react-loadable-manifest.js', |
There was a problem hiding this comment.
h: We need to ensure these are not too widely applied. The way it is now, these ignore patterns are also passed to the server source maps which might lead to false positives and ignore files we don't want to ignore.
We need a way to distinguish between which patterns we pass to which commands.
There was a problem hiding this comment.
I reverted the changes for all framework, polyfills etc. and just kept the manifest ones for now. We need to investigate if we can split up the upload calls or be smarter here. Also with turbopack there are likely more files we can ignore
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| '**/route_client-reference-manifest.js', | ||
| '**/middleware-react-loadable-manifest.js', | ||
| '**/vendor/**', | ||
| ]); |
There was a problem hiding this comment.
Missing integration or E2E test for feature PR
Low Severity · Bugbot Rules
This feat PR changes the sourcemap ignore pattern behavior (merging user patterns with defaults instead of replacing) but only includes unit tests. Per the review rules: "When reviewing a feat PR, check if the PR includes at least one integration or E2E test. If neither of the two are present, add a comment, recommending to add one." An integration test verifying the actual sourcemap upload/ignore behavior with the new merged patterns would help ensure the feature works correctly in real build scenarios.
Prevent potentially large number of noisy warnings that by design never emit sourcemaps. For example: ``` ~/b0f41f6b-e36f-409b-817f-921f7d295a7b-155.js (no sourcemap found, debug id b0f41f6b-...) - warning: could not determine a source map reference (Could not auto-detect referenced sourcemap for ~/b0f41f6b-...-155.js) ``` This cannot be safely avoided using the file path glob approach, as seen in #18938, because the file paths in question here (uuids and alphanumeric hashes) can't be guaranteed to _never_ hold user code as seen in the cases covered by #18938. Instead, when no symbolication can be performed because the chunk is a stub without any sourcemap, downgrade the warning to a debug log. fix: #22199 fix: JS-3006
closes #18872