-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Use prepare script to generate lib on install from git #1751
Conversation
Using `prepare` step to build ensures that dependency can be installed directly from git repository https://docs.npmjs.com/misc/scripts#prepublish-and-prepare `prepublishOnly` is run **after** `prepare` script so this should have no effect.
Thanks for this pull request! I like the idea but I do not like how messy our npm scripts are getting. (This is far from a criticism of this pull request, the number of scripts we have has been too many for a while now but this pull request has simply highlighted that fact.) I think a better solution to this problem would be to find a way to run a local copy of mathjs without needing to transpile the build files. That should be possible in node 13 (https://nodejs.org/api/esm.html) but we may have to make some changes to mathjs to make it work. I would be much more keen on a patch that got native esm working in node. Anyway, thanks for this patch and sorry for my brain dump. What do you think @josdejong? |
I'm with you on the messiness of the Personally, I've started using a whole extra folder on my repositories to store the actual scripts that I use. Much more control. Often better cross platform support too. In any case, I still think it should be trivial to install from git directly. The naive way of doing this would be to include the compiled sources in the git repo, which it looks like this package used to do. This problem got exacerbated by the poor documentation and a whole lot of sub-par community partial solutions. |
Thanks Cameron. Can you explain a bit more about your use case? Why would you want "to use a github version of this package as a dependency in other projects directly."? In the past we where forced to commit the generated bundles in the repository too, which is a disaster. This was needed to allow support for bower which simply publishes repositories (source code) as library. I do like a clear separation between source code and a generated library bundle. |
Specifically, I wanted to try the #1741 PR for myself but could not since it is not published to NPM. Had it had the changes I'm suggesting, I would have been able to simply run:
Instead, I had to merge my changes with their changes so that I could try their implementation before it gets accepted by your repository and published to npm. Basically, it's the official way to enable installing "compiled" version of the code to If you'd like to read more, there is a huge issue on the topic: npm/npm#3055 (comment) |
That's indeed very convenient. Are there any downsides to using |
I have a down side I think. The current workflow:
I worry that with this change we instead have:
|
@harrysarson Have you tested this? I don't think it will "work" after just running "Clone". It does not run the build unless However, thanks for bringing this up. Contrary to my previous statement, there is one small difference I'm seeing: running Previous workflow
New workflow
The only "downside" I'm seeing is that if you ever run |
Thanks Harry I understand your point. I think though that it mostly moves the "WTF" point where you probably have to check the docs, but I don't think it's really a regression. And on the contrary it has a better out of the box experience I think. Only think I can come up with is that when you've committed a feature branch that you're working on which has a broken build, and you try to check it out, stuff is broken (which makes sense). I run @harrysarson and @cinderblock shall we simply give it a try and see how it work out? |
Lets merge! I have come to think the issue I have is tangential to this. I will gro away and so if I can come up with a patch to help us avoid the need for build scripts on recent nodejs releases. If it is possible then that is the best path to making contribution straightforward. In the meantime this patch is a positive step towards it being easier to contribute to mathjs. |
👍 merging now |
I've created a script |
Published now in v6.6.2, sorry for the delay |
Using
prepare
step to build enables using a github version of this package as a dependency in other projects directly.https://docs.npmjs.com/misc/scripts#prepublish-and-prepare
Specifically, one should be able to run
npm install https://github.com/josdejong/mathjs
and immediately start using the package. As it stands, one needs to do a few extra steps to use:cd node_modules/mathjs npm install npm run build
However, if the repo is configured "correctly" the manual steps after the install are completely superfluous. Npm automatically installs devDependencies needed for build too! Try installing my fork from git to see that it "just works™":
npm i https://github.com/cinderblock/mathjs
.