Skip to content
This repository was archived by the owner on Oct 15, 2020. It is now read-only.

chakrashim: add warning for ignored engine flags#502

Merged
kfarnung merged 2 commits intonodejs:masterfrom
kfarnung:v8flags
Mar 27, 2018
Merged

chakrashim: add warning for ignored engine flags#502
kfarnung merged 2 commits intonodejs:masterfrom
kfarnung:v8flags

Conversation

@kfarnung
Copy link
Copy Markdown
Contributor

@kfarnung kfarnung commented Mar 24, 2018

  • Added a warning for any ignored engine-specific flags.
  • Added --max-old-space-size= to the list of ignored flags.

Fixes #486

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

startsWith(arg, "--nolazy") ||
startsWith(arg, "--max-old-space-size=")) {
// Ignore some flags to reduce unit test noise
fprintf(stderr, "Warning: Ignored engine flag: %s\n", arg);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This doesn't seem to bother any of the unit tests, but I'll have to check that VS Code doesn't get upset about it (I doubt it). I'd say we either have a warning for all of these flags or none of them.

@kfarnung
Copy link
Copy Markdown
Contributor Author

On Windows at least the react-scripts-ts build made a mess of the warning, but I'm not really worried about it right now:

> react-scripts-ts build

Failed to load tsconfig.json: Missing baseUrl in compilerOptions
Creating an optimized production build...
Found no baseUrl in tsconfig.json, not applying tsconfig-paths-webpack-plugin
Found no baseUrl in tsconfig.json, not applying tsconfig-paths-webpack-plugin
Warning: Unsupported engine flag:Starting type checking and linting service...
 --Using m1 workera with x2048MB- memory limit
old-space-size=2048
ts-loader: Using [email protected] and D:\Samples\my-app\tsconfig.json
Compiled successfully.

File sizes after gzip:

  35.45 KB  build\static\js\main.ceb68587.js
  300 B     build\static\css\main.29266132.css

The project was built assuming it is hosted at the server root.
To override this, specify the homepage in your package.json.
For example, add this to build it for GitHub Pages:

  "homepage" : "http://myname.github.io/myapp",

The build folder is ready to be deployed.
You may serve it with a static server:

  npm install -g serve
  serve -s build

The good news is that npm start and npm run build both worked without issues with this change.

@kfarnung kfarnung self-assigned this Mar 24, 2018
@kfarnung
Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
Contributor

@sethbrenith sethbrenith left a comment

Choose a reason for hiding this comment

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

:shipit:

@kfarnung
Copy link
Copy Markdown
Contributor Author

kfarnung commented Mar 26, 2018

@kfarnung
Copy link
Copy Markdown
Contributor Author

@mrkmarron @digitalinfinity I added an eslint ignore to trace_mgr.js due to a new rule about using Error objects in the lib/ directory. I believe this is the right fix here since we never let this error object leave the scope.

The error created isn't ever thrown, it's just used to get an
approximate stack.

PR-URL: nodejs#502
Reviewed-By: Seth Brenith <[email protected]>
* Added a warning for any ignored engine-specific flags.
* Added `--max-old-space-size=` to the list of ignored flags.

PR-URL: nodejs#502
Reviewed-By: Seth Brenith <[email protected]>
@kfarnung kfarnung merged commit d50069c into nodejs:master Mar 27, 2018
@kfarnung kfarnung deleted the v8flags branch March 27, 2018 02:30
kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request Mar 28, 2018
* Added a warning for any ignored engine-specific flags.
* Added `--max-old-space-size=` to the list of ignored flags.

PR-URL: nodejs#502
Reviewed-By: Seth Brenith <[email protected]>
kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request Apr 6, 2018
The error created isn't ever thrown, it's just used to get an
approximate stack.

PR-URL: nodejs#502
Reviewed-By: Seth Brenith <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

react-scripts-ts module tries to use unsupported option --max-old-space-size

2 participants