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

Support npm 5 prepare script for installing non-published references #1347

Open
intellix opened this issue May 12, 2018 · 4 comments
Open

Support npm 5 prepare script for installing non-published references #1347

intellix opened this issue May 12, 2018 · 4 comments

Comments

@intellix
Copy link

intellix commented May 12, 2018

npm 5 added the ability to specify a prepare script inside package.json. It's basically how to prepare a package before you want to publish it (build it).

When a package has a prepare script and you depend on a git source, npm will:

  1. Install it's devDependencies
  2. Run the prepare script

With it, you can depend directly on git sources without the need to publish them to npm first, which means you can depend on commits, branches or tags from a repository. For example:

"graphql": "github:graphql/graphql-js#some-new-branch"

Whenever I do a PR, it's because I need that code now and want to reference it straight away to start using it, so I typically reference my fork like: github:intellix/graphql-js#some-fix

Quote about prepare from npm docs:

prepare: Run both BEFORE the package is packed and published, and on local npm install without any arguments (See below). This is run AFTER prepublish, but BEFORE prepublishOnly.

It can essentially be a copy of the build command:

"prepare": "npm run build"

Spoke a little about this in: #1340

@IvanGoncharov
Copy link
Member

Whenever I do a PR, it's because I need that code now and want to reference it straight away to start using it, so I typically reference my fork like: github:intellix/graphql-js#some-fix

I have the exact same use case, I maintain my own fork of graphql-js to be used in my projects, e.g.:
https://github.com/APIs-guru/graphql-faker/blob/0c1dd3814c8bb5ad1582d3ad736b6f91696de0ab/package.json#L48

Main problem with this feature is that prepare script should work on ALL suported node versions and across ALL enviroments.
In reality it mean prepare should work on Windows and we can't use shell scripts anymore.

I'm personally was surprised on how easy it's to break build on Windows, e.g. you can't defined env variable using VAR=VALUE command and instead you need to use cross-env.

That's why we will definetly need CI test that ensure prepare on Windows.

So even though I personaly would love to use such feature I think it requires too much maintaince.

@mjmahone
Copy link
Contributor

Is this significantly different from relying on the npm branch? That branch is automatically updated after any pull request is merged and all tests pass.

@intellix
Copy link
Author

intellix commented May 14, 2018

The difference is that the npm branch is only master, and since npm5 you can point to any branch or commit from any fork, so once you submit a pull-request, it's instantly dependable without any hack

@jaydenseric
Copy link

@intellix wow how did I not realise this before, about using prepare to enable Git dependencies, thanks! There have been times I tried installing a fork/PR as a Git dependency and gave up, most recently with a graphql-relay PR.

I've begun rolling it out for all my packages, starting with graphql-api-koa.

Off topic, but that package is probably also a good reference for using Babel v7 with --keep-file-extension to distribute .mjs files, avoiding the nasty renaming and moving shell script. It's a good thing to have the source files use the right .js or .mjs extensions. Something that doesn't seem to be talked about much is using ESLint overrides config to make sure .mjs and .js files are parsed with the right sourceType and to ensure require and import syntax is only used in the correct environments. The more aligned source code is with dist output the better.

@IvanGoncharov IvanGoncharov added this to the v15.0.0 milestone Jan 31, 2019
IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this issue May 2, 2019
@IvanGoncharov IvanGoncharov removed this from the v15.x.x milestone Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants