Skip to content
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

Ensure escaped string are parsed in NODE_OPTIONS #65046

Merged

Conversation

timneutkens
Copy link
Member

@timneutkens timneutkens commented Apr 25, 2024

What?

Ensures paths that have spaces in them in NODE_OPTIONS are handled. An example of that is VS Code's debugger which adds:

--require "/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/ms-vscode.js-debug/src/bootloader.js"

Currently the output is cut off and causes: invalid value for NODE_OPTIONS (unterminated string).

Related issue: #63740

Closes NEXT-3226

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @timneutkens and the rest of your teammates on Graphite Graphite

@ijjk
Copy link
Member

ijjk commented Apr 25, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
buildDuration 15.8s 14.3s N/A
buildDurationCached 7.8s 7.5s N/A
nodeModulesSize 359 MB 359 MB ⚠️ +22.1 kB
nextStartRea..uration (ms) 386ms 390ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
139-HASH.js gzip 31.8 kB 31.8 kB N/A
2478adb-HASH.js gzip 53.5 kB 53.5 kB N/A
4967-HASH.js gzip 5.1 kB 5.1 kB
6701.HASH.js gzip 168 B 168 B
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 228 B 223 B N/A
main-HASH.js gzip 31.6 kB 31.6 kB N/A
webpack-HASH.js gzip 1.65 kB 1.64 kB N/A
Overall change 50.4 kB 50.4 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
_app-HASH.js gzip 194 B 193 B N/A
_error-HASH.js gzip 192 B 192 B
amp-HASH.js gzip 511 B 512 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 2.52 kB 2.52 kB N/A
edge-ssr-HASH.js gzip 266 B 265 B N/A
head-HASH.js gzip 365 B 362 B N/A
hooks-HASH.js gzip 391 B 390 B N/A
image-HASH.js gzip 4.32 kB 4.31 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.69 kB 2.69 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 393 B 395 B N/A
withRouter-HASH.js gzip 323 B 323 B
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 1.22 kB 1.22 kB
Client Build Manifests
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
_buildManifest.js gzip 481 B 483 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
index.html gzip 528 B 529 B N/A
link.html gzip 540 B 540 B
withRouter.html gzip 523 B 524 B N/A
Overall change 540 B 540 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
edge-ssr.js gzip 108 kB 108 kB N/A
page.js gzip 3.05 kB 3.05 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
middleware-b..fest.js gzip 624 B 622 B N/A
middleware-r..fest.js gzip 155 B 154 B N/A
middleware.js gzip 27.9 kB 27.9 kB N/A
edge-runtime..pack.js gzip 839 B 839 B
Overall change 839 B 839 B
Next Runtimes
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
app-page-exp...dev.js gzip 171 kB 171 kB
app-page-exp..prod.js gzip 98.4 kB 98.4 kB
app-page-tur..prod.js gzip 99.9 kB 99.9 kB
app-page-tur..prod.js gzip 94.2 kB 94.2 kB
app-page.run...dev.js gzip 157 kB 157 kB
app-page.run..prod.js gzip 93 kB 93 kB
app-route-ex...dev.js gzip 21.4 kB 21.4 kB
app-route-ex..prod.js gzip 15.2 kB 15.2 kB
app-route-tu..prod.js gzip 15.2 kB 15.2 kB
app-route-tu..prod.js gzip 14.9 kB 14.9 kB
app-route.ru...dev.js gzip 21.3 kB 21.3 kB
app-route.ru..prod.js gzip 14.9 kB 14.9 kB
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB
pages-turbo...prod.js gzip 21.4 kB 21.4 kB
pages.runtim...dev.js gzip 22.1 kB 22.1 kB
pages.runtim..prod.js gzip 21.4 kB 21.4 kB
server.runti..prod.js gzip 65.3 kB 65.3 kB
Overall change 975 kB 975 kB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS Change
0.pack gzip 1.61 MB 1.62 MB ⚠️ +1.92 kB
index.pack gzip 112 kB 113 kB ⚠️ +1.13 kB
Overall change 1.72 MB 1.73 MB ⚠️ +3.05 kB
Diff details
Diff for main-HASH.js

Diff too large to display

Commit: 985298c

@ijjk
Copy link
Member

ijjk commented Apr 25, 2024

Tests Passed

Copy link
Contributor

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. better than using a regex. I'd prefer if this functionality was exposed by Node but its not so this is good solution for now.

@wyattjoh wyattjoh force-pushed the 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS branch from e0367d4 to 7130026 Compare April 25, 2024 17:04
@wyattjoh wyattjoh force-pushed the 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS branch from c02c495 to 985298c Compare April 25, 2024 23:32
@timneutkens timneutkens merged commit ae1fe56 into canary Apr 26, 2024
9 of 10 checks passed
@timneutkens timneutkens deleted the 04-25-Ensure_escaped_string_are_parsed_in_NODE_OPTIONS branch April 26, 2024 08:56
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants