-
Notifications
You must be signed in to change notification settings - Fork 3.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
feat: Add prefetch
script to install and ci commands
#5264
Conversation
prefetch
script to install and ci commandsprefetch
script to install and ci commands
Please fix the issue. |
when will this be merged? |
Changes like this need to be an RFC first. |
@ljharb It seems like there already is one to do nearly exactly the same thing (npm/rfcs#403), but nothing has been done on it in over a year. Is there something I can do to get this issue fixed? |
Nope. Looks like #2660 Is the issue to watch. |
It would seem that the issue needs a kickstart. There has been no activity on that issue either. Not even an update on the plan. As I mentioned in my comment there, and here, this change in behavior was never documented and so it would seem to be a critical bug that is not being fixed. An update of some kind would be greatly appreciated. |
3037d35
to
f3b0c43
Compare
As @ljharb stated, this kind of a change needs to go through the rfc process. I know it can be frustrating having to wait on changes like this but we are trying to be very cautious with the lifecycle scripts. |
But, you were not careful with the lifecycle scripts. That's why this breaking change-bug exists. I apologize for being so direct but no one seems to be acknowledging this crucial point. |
In v7 the
preinstall
script was intentionally changed to run after dependencies are installed. This was never documented as a breaking change and has since then been neither fixed nor otherwise remedied. While I personally would argue that since it was never listed as a breaking change it should be considered a bug, I recognize that the time elapsed since then complicates matters.Because of that, I propose simply adding a new script that will run before dependencies are installed to restore a critical piece of functionality that was removed without being documented. An oft-cited use for a script being run at that point is to authenticate to a private registry before attempting to fetch packages.
I also recognize this issue may be more complicated than my initial single commit attempts to make it seem, but the bug report (#2660) seems to have stagnated since it was submitted in February of 2021, and as far as I can tell no one has even attempted a fix. There is debate over whether this inaction means that it must be very complicated or not. I hope that this PR will at least spur additional debate. I'm happy to be proven wrong.
I'm largely unfamiliar with the npm codebase, so please do suggest a different script name (I chose
prefetch
fairly arbitrarily just based on my understanding, but have no attachment to it), different placement for therunScript
call etc., or additional required changes/tests I may have missed. Or if someone with more understanding of the details is inspired to pick this up and run with it, by all means, please do so.Edit: It looks like there was an early attempt (#2713) to move
preinstall
beforereify
that was closed because it would be "breaking" (even though, again, this would actually just fix an undocumented change), but no alternative remedy was ever acted on.References
Fixes #2660
Related to #2713
Related to npm/rfcs#403