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

Fixes assertion #628

Merged
merged 3 commits into from
Dec 11, 2019
Merged

Fixes assertion #628

merged 3 commits into from
Dec 11, 2019

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Dec 11, 2019

What's the problem this PR addresses?

Fixes #626

I found the issue - we're currently iterating over each package to find out their linkers, then we iterate a second time to inject the packages into the linkers. When I say "each package" it's not entirely true: we don't actually do this for packages that are "virtual package templates", since they usually don't need to be linked.

The problem occurs when such a regular package template somehow depends on a virtual package template: since the virtual package template didn't get traversed in step 1, we haven't assigned it a linker, hence the failed assertion.

Package templates should never be valid package dependencies, but they currently are in one particular case: when a package lists the same name as both a dependency and a peer dependency. In this case the regular dependency is still resolved as part of the regular resolution pass, but then we simply ignore the regular resolution to prefer the peer resolution instead. This makes the package orphan, so it never goes into the virtual hydratation, and its virtual package template dependencies never get changed into virtual instances.

How did you fix it?

  • First I will add proper support for listing the same name as both dependency and peer dependency. The semantic will be that Yarn will prefer the peer dependency if possible, but will fallback to the regular dependency otherwise. It'll be an optional dependency with steroids.

  • Second I will make the package hydratation process keep track of which packages have been traversed, and use that when linking the project (rather than all the packages currently stored). I won't prune them altogether because we will still want the resolutions to be persisted into the lockfile.

@arcanis arcanis merged commit 18aa061 into master Dec 11, 2019
@arcanis arcanis deleted the mael/assert-fail-vpt branch December 11, 2019 13:24
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

Successfully merging this pull request may close these issues.

[Bug] jest-runner-eslint triggers an assertion failed
1 participant