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

Replacing prepublish script with prepare. #112

Merged
merged 1 commit into from
Oct 16, 2017
Merged

Conversation

max-b
Copy link
Contributor

@max-b max-b commented Oct 10, 2017

If someone wanted to install this from github instead of npm (say because they wanted a version that hadn't been published to npm), the prepublish script won't run and they'll be left without the actual built lib files.

See:
npm/npm#3055
http://blog.npmjs.org/post/161081169345/v500
https://twitter.com/maybekatz/status/860363896443371520

@goldhand
Copy link
Owner

This kind of sucks on npms part. prepare was introduced in npm@4 and I don't think they plan on it being backwards compatible. What if someone is using npm<=3? Any ideas?

@max-b
Copy link
Contributor Author

max-b commented Oct 16, 2017

So my understanding is that prepare will still run when you publish new releases to npm. So you'll still pre-compile assets on publish and users of npm<=3 will still be able to npm install your package normally. What users of npm<=3 won't be able to do is npm install this library from github because the prepare script won't run and compile their assets on installation. But that's the same state that the library currently is in - it's impossible to npm install this library from github because it won't compile. Short of checking in compiled assets (the lib folder), which I guess I'm sort of philosophically against, I'm not sure there's any way to allow all users to install this library from github.

@goldhand goldhand merged commit b938331 into goldhand:master Oct 16, 2017
@goldhand
Copy link
Owner

I understand, thanks!

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.

2 participants