-
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
[BUG] Prepare scripts in workspaces should wait for dependencies #3034
Comments
All of the lifecycle scripts have a So, to be clear, before |
If you would like to propose a more formal solution for this problem that allows for more complex situations to be addressed, the RFC process is a good place to suggest something: https://github.com/npm/rfcs |
Preprepare does not solve my actual problem because I have 6 packages in my workspace and a nesting depth of 3. I can make a rfc but I feel like it's more of a bug than a new feature, npm should be running the scripts for child dependencies before the parents. |
@wraithgar Would like to reiterate this really is a bug and currently I believe this makes it impossible to use typescript in a workspace since you can't ensure a package is actually built before something tries to use it. |
Sorry, didn't see that this was specifically related to workspaces when I replied.
|
You can follow the progress of adding workspace support to all of arborist's reify commands in the cli here |
@wraithgar What does that mean? It's in the docs: https://docs.npmjs.com/cli/v7/using-npm/workspaces#installing-workspaces |
You're right, looks like install is already pulling in workspaces, which is different than the This is already on our radar but I'll reopen this so it's being explicitly represented here in this issue too. |
Adds the ability for lifecycle scripts of Link nodes to depend on each other, such that a `prepare` script of linked module A can make use of files that are a result of a `prepare` script of a linked package B. This is important in order to unlock workflows in which a workspace needs to make use of another workspace but they all have transpilation steps (very common in Typescript projects) so tracking the order (and completion) in which the dependencies lifecycle scripts runs becomes necessary. Fixes: npm/cli#3034
Adds the ability for lifecycle scripts of Link nodes to depend on each other, such that a `prepare` script of linked module A can make use of files that are a result of a `prepare` script of a linked package B. This is important in order to unlock workflows in which a workspace needs to make use of another workspace but they all have transpilation steps (very common in Typescript projects) so tracking the order (and completion) in which the dependencies lifecycle scripts runs becomes necessary. Fixes: npm/cli#3034
Adds the ability for lifecycle scripts of Link nodes to depend on each other, such that a `prepare` script of linked module A can make use of files that are a result of a `prepare` script of a linked package B. This is important in order to unlock workflows in which a workspace needs to make use of another workspace but they all have transpilation steps (very common in Typescript projects) so tracking the order (and completion) in which the dependencies lifecycle scripts runs becomes necessary. Fixes: npm/cli#3034
In the short term, you can force serialized operation by doing More generally, this does get a bit tricky in the case where the dependency graph cycles back on itself. For example, |
Hm, exploring a bit further, there is really no way to do solve this bootstrapping problem in general that doesn't lead to either a traveling salesman problem or deadlocks. Consider this dependency graph:
Let's say that:
This is unresolvable. Picking any arbitrary point in the graph, we end up with a failure for at least 2 reasons:
Furthermore, even if we say we just won't handle cycles in this way, sorting by any combination of dependency relationships or depth within the dependency graph as a heuristic doesn't really work either. Take for example the
When we look at the
So, if we have Even if we run package scripts in serial like npm v6 did (and which you can do with the Simply put, there is no way we can guarantee, or ever could have guaranteed, that a package's dependencies will have their The safest assumption is that lifecycle events happen in a single platonic moment for all packages within the tree, and design your modules accordingly. |
Thanks for the workaround with But I must say that the fact that cycles can merely be represented has not often been sufficient reason to give up on something like this.
I have had a different experience with workspaces, where it's rather not by accident that we have non-cyclic dependency graphs among workspaces.
It might even be reasonable to suppose that:
and thus anything downstream would be needed, as probably as direct dependencies. I'd be happy to have Using the ordering from Ordered prepare for projects with cyclic dependencies seems better too. If there is some permutation that would work, I'd prefer to have Doing them all at once is faster for projects with packages that can be prepared independently, but among projects that have set up workspaces, I really don't know how common that is. Doing The behavior of npm running the prepare scripts from the top level workspace is relatively new, first added in 7.20.0. Right now would be the best opportunity to make ordering like |
Current Behaviour:
It seems that prepare scripts are now run in parallel, which means that if:
A
depends onB
B
'sprepare
script takes some time. e.g. runstsc
A
usesB
in its prepare script e.g. runs owntsc
which requiresB
to be built firstThen
A
'sprepare
script will fail becauseB
's has not completed.If I switch the package.json scripts from
prepare
toprepublish
then it seems to work without a problem.Is there an alternative lifecycle script to the deprecated
prepublish
?This may be an
arborist
issue.Expected Behaviour:
npm install
should wait for dependent'sprepare
scripts to complete before running.Steps To Reproduce:
I've tried to capture a reproduction here:
https://github.com/timoxley/npm7-prepare-issue
Environment:
The text was updated successfully, but these errors were encountered: