-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Prevent npm@6-related Netlify failures on release-3.6
branch
#9112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For the time being, the documentation team is sticking to npm@6 for managing the `docs/` directories of our various projects, which mostly entails sticking with `lockfileVersion:1` in `docs/package-lock.json`, instead of `lockfileVersion:2` (like `./package-lock.json`). Apollo Client itself welcomes contributors using the latest versions of Node.js and npm, so we've embraced `lockfileVersion:2` in the root `package-lock.json` file, even defending it from reversion with checks like #9068. However, when Netlify attempts to run npm@6 in the root directory of the apollo-client repository, the newer `lockfileVersion:2` in `package-lock.json` causes failures on `npm install` such as https://app.netlify.com/sites/apollo-client-docs/deploys/619d49161cd5b80007973178 Reading over PR #9006 where the `cd .. && npm i && npm run build` commands were introduced, it doesn't seem to be absolutely necessary to build Apollo Client itself when building the docs, so I propose we fix the Netlify build failures by running `npm i` with appropriate npm versions in each directory during the Netlify build, and not running `npm run build` in the root apollo-client directory.
I don't think building Apollo Client itself should be necessary for Netlify to build the docs, but it doesn't hurt, and I don't want to risk breaking the docs in some unforeseen way.
benjamn
commented
Nov 23, 2021
hwillson
approved these changes
Nov 23, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome @benjamn - thanks!
benjamn
added a commit
that referenced
this pull request
Nov 23, 2021
benjamn
added a commit
that referenced
this pull request
Feb 4, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For the time being, the documentation team is sticking to npm@6 for managing the
docs/
directories of our various projects, which mostly entails sticking withlockfileVersion:1
indocs/package-lock.json
, instead oflockfileVersion:2
(like./package-lock.json
).Apollo Client itself welcomes contributors using the latest versions of Node.js and npm, so we've switched to
lockfileVersion:2
in the rootpackage-lock.json
file, even defending it from reversion with checks like #9068.However, when Netlify attempts to run npm@6 in the root directory of the
apollo-client
repository, the newerlockfileVersion:2
inpackage-lock.json
causes failures onnpm install
like this one.Reading over PR #9006 where theI propose we fix the Netlify build failures by runningcd .. && npm i && npm run build
commands were introduced, it doesn't seem to be absolutely necessary to build Apollo Client itself when building the docs, sonpm i
with appropriate npm versions in each directory during the Netlify build,and skip runningnpm run build
in the rootapollo-client
directory (unless @hwillson or @brainkim or anyone else can think of a reason that's necessary for building the docs).