fix(package-graph): ensure to touch all nodes #3234
Conversation
There was a problem hiding this comment.
Thanks @simllll, is there a failing test we can add first and fix with this?
core/package-graph/index.js
Outdated
| alreadyVisited.add(topLevelDependent); | ||
| const identifier = baseNode.name + ':' + topLevelDependent.name; | ||
| if (alreadyVisited.has(identifier)) { | ||
| console.log('topLevelDependent', identifier); |
There was a problem hiding this comment.
@simllll Please remove the console log
There was a problem hiding this comment.
oha, sorry about that
|
Okay, I was trying to break it down further, and I guess I found the real issue here: a dependency circle caused this behavior. Even though the circle is unrelated to this package. So it seems it's not really a bug, it's just a hard to find "issue". Sorry for the troubles |
|
I ran into the same thing today again, but now I don't have any circular dependencies.. still not sure what's going on (lerna 5.2.0), but it's for sure that even though the package list on |
|
@simllll are you by any chance using the new |
|
Hi @ghiscoding , nope, not using this option |
|
so does this PR fix your latest issue then? if so I'd be willing to push it to Lerna-Lite |
|
@simllll I can confirm that #2874 is causing quite a negative side effect, we ran into this problem on a large project which had 43 packages changed, but only returned 5 of them were returned and that caused a lot of packages to not be published when they should have. Your code change seems to fix the issue, so I'll apply this code change into Lerna-Lite unless something better comes up. Thanks for that |
|
Based on the feedback above let's get this merged, we'll need to spend some more time trying to understand these changes better though and have some test coverage. If there are any other issues that get raised then we will likely have to revert the original change |
Description
the fix of not touching a node twice (#2874) has a side effect so that not all scirpts are executed anymore.
this fixes it by still improving / keeping the performance benefits of the other pr.
fixes #3233