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

Fix npm@6 warnings about deprecated "prepublish" #1340

Merged
merged 1 commit into from
May 11, 2018

Conversation

IvanGoncharov
Copy link
Member

NPM 6 produces this warning:

npm WARN prepublish-on-install As of npm@5, `prepublish` scripts are deprecated.
npm WARN prepublish-on-install Use `prepare` for build steps and `prepublishOnly` for upload-only.
npm WARN prepublish-on-install See the deprecation note in `npm help scripts` for more information.

Yarn/NPM support new prepublishOnly property:

As of npm@4.0.0, a new event has been introduced, prepare, that preserves this existing behavior. A new event, prepublishOnly has been added as a transitional strategy to allow users to avoid the confusing behavior of existing npm versions and only run on npm publish (for instance, running the tests one last time to ensure they're in good shape).

Currently NPM deploy done using:

node_js: '8'

which use NPM 5.

@mjmahone mjmahone merged commit 6d7e08b into graphql:master May 11, 2018
@IvanGoncharov IvanGoncharov deleted the prepublishOnly branch May 11, 2018 20:32
@IvanGoncharov
Copy link
Member Author

@mjmahone Thanks.

@mjmahone
Copy link
Contributor

@IvanGoncharov did my squash-and-merge give you the proper attribution? I want to make sure I'm not rewriting pull-request owners somehow, and that you're getting due credit for anything we pull in! If I did mess things up, I'm sorry and will happily change my merge flow.

Also thank you for this fix!

@IvanGoncharov
Copy link
Member Author

@mjmahone Everything is fine it's look like this:
image
Plus GitHub counts me as an author so I still have a chance to beat greeenkeeper[bot] one day 😆
image

@intellix
Copy link

Maybe it'd be good to support npm prepare script which can just be a reference to npm build

@IvanGoncharov
Copy link
Member Author

@intellix Why would you need to build dist folder on npm install?
If you want to try master there is a special branch for that:
https://github.com/graphql/graphql-js#want-to-ride-the-bleeding-edge

Or do you need it for something else?

@intellix
Copy link

intellix commented May 12, 2018

It'd support bleeding edge without requiring a special branch. You could install any branch like: github:graphql/graphql-js#whatever

It won't build a dist upon npm install, it'll just support the above without any consequence or complexities. It's new since npm 5

@IvanGoncharov
Copy link
Member Author

@intellix I wrote a pretty long response but I don't want it to be lost inside merged PR with an unrelated title.
Can you please open an issue for that feature?

@IvanGoncharov
Copy link
Member Author

@intellix Short version: for this to work we need to make npm run build Windows-friendly which is a big problem on its own.

@leebyron
Copy link
Contributor

woohoo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants