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

Docs: externalNpm does not need to execute npm? #573

Open
raquo opened this issue Oct 5, 2023 · 1 comment
Open

Docs: externalNpm does not need to execute npm? #573

raquo opened this issue Oct 5, 2023 · 1 comment

Comments

@raquo
Copy link

raquo commented Oct 5, 2023

Hi, just a minor suggestion – the description and documentation for externalNpm says to include the invocation of the npm command before returning the directory in which package.json and node_modules are located, however as far as I can tell, invoking the npm command there does not do anything except print a long npm help message on every compile.

On the other hand, unlike npm, running the yarn command without any arguments actually installs the dependencies. If the intent was for externalNpm to install dependencies on every compile, then it should include npm install command, not just npm. That seems a bit excessive imho, but if that's the intent, the command should be updated and the docs should briefly explain why it's needed in the first place, because e.g. the official scala.js tutorial does not invoke npm / yarn, and it seems to work fine.

@oyvindberg
Copy link
Collaborator

Thanks for bringing this up - it's a good point!

The intention was absolutely that externalNpm should always install dependencies, all the time.
I used and still use yarn, which can check if node_modules is up to date in milliseconds, so that has always been fast.

I know npm install has always been slower when there is no work to be done, so I understand it's an irritation and people end up calling it manually. That's a pragmatic solution, and it means externalNpm can just return a directory.

I can try to describe this better in the docs 👍

Future

I understand yarn falling out of favour though, so I guess it's time for this to get a default implementation which calls npm probably.

It would be great to use sbt's caching mechanism with the contents of package.json to determine if it should run maybe. I likely won't work on it for a while, but if anybody wants to try I'll gladly help out

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

No branches or pull requests

2 participants