feat: Add prefetch script to install and ci commands#5264
feat: Add prefetch script to install and ci commands#5264bearfriend wants to merge 1 commit intonpm:latestfrom
prefetch script to install and ci commands#5264Conversation
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
preinstallscript 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
prefetchfairly arbitrarily just based on my understanding, but have no attachment to it), different placement for therunScriptcall 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
preinstallbeforereifythat 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