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

Upgrade auto-installed Node to latest LTS and suppress some npm install errors #8956

Merged
merged 6 commits into from Sep 9, 2020

Conversation

joheriks
Copy link
Contributor

@joheriks joheriks commented Sep 7, 2020

This PR:

  1. Upgrade Node to latest LTS version (12.18.3)
  2. Add node-gyp as a dev dependency (required for native compilation of fsevents 1 on macOS). Note that fsevents 1 is deprecated and is only required for chokidar 2 (also deprecated), which is a requirement of webpack 4 but is not used at runtime (chokidar 3 is included). Hence, fix 2 is only cosmetic: it prevents the unsightly spawn node-gyp EACCES errors, but on the flip side the native compilation increases the npm install time compared to the install script failing. All for no good reason.
  3. Add parameter --scripts-prepend-node-artifact=true to Node install to ensure that the same node executable is used for running npm and package install scripts.

Related to #8741.

@joheriks joheriks added this to Iteration Reviews in OLD Vaadin Flow ongoing work (Vaadin 10+) via automation Sep 7, 2020
@joheriks joheriks force-pushed the joherks/8741-npm-install-errors branch from 612fe5a to 1dd3c20 Compare September 7, 2020 12:09
@joheriks joheriks changed the title Upgrade Node to latest LTS and suppress some npm install errors Upgrade auto-installed Node to latest LTS and suppress some npm install errors Sep 8, 2020
@joheriks joheriks force-pushed the joherks/8741-npm-install-errors branch from ecbd116 to 4d95ba4 Compare September 8, 2020 09:01
@vaadin-bot vaadin-bot added +1.0.0 and removed +0.0.1 labels Sep 8, 2020
@joheriks joheriks force-pushed the joherks/8741-npm-install-errors branch from 4d95ba4 to fed5e81 Compare September 8, 2020 11:58
@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 7 issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. CRITICAL FlowModeAbstractMojo.java#L146: Move this variable to comply with Java Code Conventions. rule
  2. CRITICAL FlowModeAbstractMojo.java#L157: Move this variable to comply with Java Code Conventions. rule
  3. CRITICAL TaskRunNpmInstall.java#L273: Reduce the number of conditional operators (4) used in the expression (maximum allowed 3). rule
  4. MAJOR FrontendTools.java#L300: Rename "nodeVersion" which hides the field declared at line 151. rule
  5. MINOR FlowModeAbstractMojo.java#L74: Remove this use of "SERVLET_PARAMETER_INITIAL_UIDL"; it is deprecated. rule
  6. MINOR FlowModeAbstractMojo.java#L112: Remove this use of "SERVLET_PARAMETER_ENABLE_PNPM"; it is deprecated. rule
  7. MINOR FlowModeAbstractMojo.java#L121: Remove this use of "REQUIRE_HOME_NODE_EXECUTABLE"; it is deprecated. rule

@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Sep 8, 2020
@denis-anisimov denis-anisimov merged commit 0bb5f12 into master Sep 9, 2020
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Iteration Reviews to Done - pending release Sep 9, 2020
@denis-anisimov denis-anisimov deleted the joherks/8741-npm-install-errors branch September 9, 2020 08:23
caalador pushed a commit that referenced this pull request Sep 10, 2020
…all` errors (#8956)

Upgrade installed node to latest LTS and set `scripts-prepend-node-artifact` to ensure same node is used to run npm and package install scripts

Related to #8741
caalador pushed a commit that referenced this pull request Sep 10, 2020
…all` errors (#8956)

Upgrade installed node to latest LTS and set `scripts-prepend-node-artifact` to ensure same node is used to run npm and package install scripts

Related to #8741
caalador pushed a commit that referenced this pull request Sep 11, 2020
…all` errors (#8956)

Upgrade installed node to latest LTS and set `scripts-prepend-node-artifact` to ensure same node is used to run npm and package install scripts

Related to #8741
joheriks pushed a commit that referenced this pull request Sep 11, 2020
…all` errors (#8956)

Upgrade installed node to latest LTS and set `scripts-prepend-node-artifact` to ensure same node is used to run npm and package install scripts

Related to #8741
pleku pushed a commit that referenced this pull request Sep 17, 2020
…all` errors (#8956)

Upgrade installed node to latest LTS and set `scripts-prepend-node-artifact` to ensure same node is used to run npm and package install scripts

Related to #8741
mshabarov pushed a commit that referenced this pull request Sep 17, 2020
…all` errors (#8956)

Upgrade installed node to latest LTS and set `scripts-prepend-node-artifact` to ensure same node is used to run npm and package install scripts

Related to #8741
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging this pull request may close these issues.

None yet

4 participants