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

Enforce minimum npm and node version #971

Merged
merged 2 commits into from May 6, 2022

Conversation

anders-kiaer
Copy link
Collaborator

@anders-kiaer anders-kiaer commented May 5, 2022

The comitted ./react/package-lock.json is switching between lockFileVersion 1 and 2 (depending on which PR author the commits come from).

We should as a project stick to one version. Since lockFileVersion 2 has been around since npm == 7 (node 16) it should be safe to require this version.

Something is not perfect with the current commited lock file in master (dependabot complains about not being able to read it). Hopefully this PR fixes that.

Note that an unmaintained devDependency needs to be removed in order to create a consistent package.json file, see #972.

Closes #877 and closes #640.

@anders-kiaer anders-kiaer force-pushed the enforcenodeversion branch 2 times, most recently from 9888baf to bdb7b8f Compare May 5, 2022 20:19
@equinor equinor deleted a comment from codecov-commenter May 5, 2022
@anders-kiaer anders-kiaer marked this pull request as ready for review May 5, 2022 20:29
@shadab-skhan
Copy link
Contributor

Thanks @anders-kiaer this was much needed

@shadab-skhan
Copy link
Contributor

shadab-skhan commented May 6, 2022

It breaks storybook, defining host in storybook script works ("storybook": "start-storybook -p 6006 -h localhost")
storybookjs/storybook#18019

@hkfb
Copy link
Collaborator

hkfb commented May 6, 2022

It breaks storybook, defining host in storybook script works ("storybook": "start-storybook -p 6006 -h localhost") storybookjs/storybook#18019

Works with node version 16

@anders-kiaer
Copy link
Collaborator Author

Perhaps I should also remove --ignore-scripts from README.md and CI commands (now that we enforce it through .npmrc). Thoughts?

@codecov-commenter
Copy link

Codecov Report

Merging #971 (0821e55) into master (d794b27) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #971   +/-   ##
=======================================
  Coverage   44.98%   44.98%           
=======================================
  Files         115      115           
  Lines        3621     3621           
  Branches      675      675           
=======================================
  Hits         1629     1629           
  Misses       1969     1969           
  Partials       23       23           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@shadab-skhan shadab-skhan merged commit 8d56b6d into equinor:master May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enforce NPM version >= 7 Add install-scripts in a .npmrc file
4 participants