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

Conversation

@kfarnung
Copy link
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
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
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 typescript@2.7.2 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
Contributor Author

Copy link
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
Contributor Author

kfarnung commented Mar 26, 2018

@kfarnung
Copy link
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 <sethb@microsoft.com>
* 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 <sethb@microsoft.com>
@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 <sethb@microsoft.com>
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 <sethb@microsoft.com>
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