Skip to content

Commit

Permalink
Fix package.json in my-app to utilize preinstall hook
Browse files Browse the repository at this point in the history
  • Loading branch information
@ryansonshine
ryansonshine committed May 17, 2021
1 parent e541f30 commit bb2c74f
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion my-app/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "A sample application consuming a private scoped npm package",
"main": "index.js",
"scripts": {
"prepare": "npm run co:login",
"preinstall": "npm run co:login",
"co:login": "aws codeartifact login --tool npm --repository my-repo --domain my-domain"
},
"dependencies": {
Expand Down

4 comments on commit bb2c74f

@tewson-brightflag
Copy link

Choose a reason for hiding this comment

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

Hi @ryansonshine. I found this repo from your blog post.

Is this preinstall script meant to run before npm tries to install private packages? I have been trying to do a similar thing but it seems preinstall is only run after dependency installation.

Here's an explanation from npm themselves.

@ryansonshine
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @tewson-brightflag !

The blog post was written with npm v6 in mind and I was unaware of this change with npm v7. Thanks for bringing this to my attention! I'll do some digging and post an update.

@tewson-brightflag
Copy link

Choose a reason for hiding this comment

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

@ryansonshine Appreciate the speedy response! 👍

@ryansonshine
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryansonshine Appreciate the speedy response!

Anytime! I'm drafting up an RFC but am still bouncing between whether or not we should make a push for this being a bug with npm.

In the interim, I'll post an update on the blog to communicate that this pattern will not work for npm v7.

Please sign in to comment.