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

remove lockfile, add .npmignore #100

Merged
merged 4 commits into from
Apr 7, 2021
Merged

Conversation

mattwelke
Copy link
Collaborator

Some cleanup:

  • Lock files should not be used for libraries (projects that are dependencies of other projects). See https://dev.to/gajus/stop-using-package-lock-json-or-yarn-lock-3ddi for more info. NPM has been ignoring this file for our publishes anyways. Not having the lockfile prevents contributors from accidentally using versions of the library dependencies that are different from the versions that would be used by projects that use this library as a dependency.
  • Add .npmignore file so we can start to omit automation tooling (like the .github directory we now have). People like clean NPM packages downloaded into their projects.

.npmignore Outdated
@@ -0,0 +1,2 @@
.github
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.github
.github/

Since .github is always a directory, we can add a slash after it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree. I tend to use .github because it works as a glob pattern, but explicit is probably better than implicit. Changed.

@Yash-Singh1
Copy link
Collaborator

I would also suggest having a .npmrc file at the root of the repository with the following:

package-lock=false

This would disable package-lock.json from installing in the first place when npm install is run.

.npmignore Outdated
@@ -0,0 +1,2 @@
.github

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are creating a .npmignore here it would be best to ignore other items such as:

test/
misc/
.travis.yml
Makefile

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True. I didn't realize until double checking the docs on .npmignore that NPM uses .gitignore if you have no .npmignore. So we need to add this stuff back in so that we continue to get the behavior we've had so far. Changed.

@mattwelke
Copy link
Collaborator Author

mattwelke commented Apr 7, 2021

@Yash-Singh1 Cool, I didn't know that was a feature of .npmrc. I agree that's desirable behavior for a library. But I can't find package-lock=false referenced in their docs on the file (https://docs.npmjs.com/cli/v7/configuring-npm/npmrc). Can you double check whether that's the right setting, and preferably link a doc showing how to use it?

@Yash-Singh1
Copy link
Collaborator

That link includes where to keep an .npmrc and what it is. Here is the documentation for how to configure with it: https://docs.npmjs.com/cli/v7/using-npm/config#package-lock.

@mattwelke mattwelke merged commit aefadde into master Apr 7, 2021
@mattwelke mattwelke deleted the tooling/remove-lockfile branch April 7, 2021 23:51
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.

None yet

2 participants