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

Packages that are both fetched from npm in the root, and packages in the workspace are not symlinked consistently #4289

Closed
loganfsmyth opened this issue Aug 31, 2017 · 15 comments · Fixed by #7289

Comments

@loganfsmyth
Copy link

loganfsmyth commented Aug 31, 2017

Do you want to request a feature or report a bug?
Bug, since it seems like this is related somehow to specifically what dependencies are in use. Hard to say though.

What is the current behavior?
Reproduction repository: https://github.com/loganfsmyth/__yarn-workspace-bug

The structure

// Root package.json has dep on:
babel-core@7.0.0-alpha.18
babel-cli@7.0.0-alpha.18

and

// Packages:
packages/babel-core@7.0.0-alpha.19
packages/babel-cli@7.0.0-alpha.19 depending on babel-core@7.0.0-alpha.19

produces

node_modules/
    babel-core@7.0.0-alpha.18
    babel-cli@7.0.0-alpha.18

There seems to be some installation issues when the root package depends on older versions of packages from npm, and they also exist in the yarn workspace packages themselves, they don't have their dependencies symlinked to the related packages.

If the current behavior is a bug, please provide the steps to reproduce.
https://github.com/loganfsmyth/__yarn-workspace-bug

What is the expected behavior?
See repo. Packages should be symlinked as would be normally expected.

node_modules/
    babel-core@7.0.0-alpha.18
    babel-cli@7.0.0-alpha.18
packages/babel-cli/node_modules/
    babel-core@7.0.0-alpha.19 -> ../../babel-core

Please mention your node.js, yarn and operating system version.
Node: 6.9.1
Yarn: 0.27.5
OS: OSX 10.12.5

@Daniel15
Copy link
Member

@BYK @arcanis Any ideas on this one? My guess was that having a different version at the root compared to in the workspace itself was confusing Yarn, but I don't know much about how workspaces work internally.

@loganfsmyth
Copy link
Author

I should clarify, this doesn't seem to be related to babel-core being in both places, but rather babel-cli. If I remove babel-cli dependency from the root package, the symlink from babel-cli to babel-core does get created.

I'm wondering, does the babel-cli in packages get skipped somehow because it's instead installing the version in the repo root?

loganfsmyth added a commit to loganfsmyth/babel that referenced this issue Sep 1, 2017
Yarn currently fails to add the correct symlinks in some cases. See yarnpkg/yarn#4289
loganfsmyth added a commit to babel/babel that referenced this issue Sep 1, 2017
Yarn currently fails to add the correct symlinks in some cases. See yarnpkg/yarn#4289
@hzoo
Copy link
Contributor

hzoo commented Oct 3, 2017

We had to revert in Babel due to some issues ^

@bestander
Copy link
Member

Sorry for not reacting fast, the team is aware of the problem

@hzoo
Copy link
Contributor

hzoo commented Oct 3, 2017

All good, we are just noticing everything is slow after moving babel-standalone into the repo 😄 to get some cool repl features and having to revert.

@BYK BYK self-assigned this Oct 3, 2017
@Daniel15
Copy link
Member

Daniel15 commented Oct 3, 2017

we are just noticing everything is slow after moving babel-standalone into the repo

lol, @hzoo you can probably blame me for that 😛 We can always move babel-standalone out of the repo, but I think the REPL functionality is very useful.

@hzoo
Copy link
Contributor

hzoo commented Oct 4, 2017

Ya, I was thinking that we should make a smaller version of babel-standalone (babel-repl, babel-standalone-repl) that only includes what we need to test PRs/repl?

I totally think it's useful and why I rushed to add it in the first place (but then now bootstrap is taking 200s instead of like 50s with workspaces 😄 and like 120 with standalone or something) hzoo/babel@32e71b6 but dono it that actually does anything lol

@Daniel15
Copy link
Member

Daniel15 commented Oct 4, 2017

Do you mean just for the plugins / presets modified in the PR? What if you want to test the interaction between the modified plugins and some other plugins? I suspect a lot of builds would want the 'full' REPL build available. Maybe we could split up the build into several files and only build the files that need to be rebuilt.

Anyways, we should move this discussion to the Babel repo 😛

@danez
Copy link
Contributor

danez commented Nov 29, 2018

This is can also be produced with non workspace dependencies like:

// Root package.json has dep on:
babel-cli@7.0.0-alpha.18
isarray@2.0.0

and

// Packages:
packages/babel-cli@7.0.0-alpha.19 depending on isarray@1.0.0

does not install isarray@1.0.0 anywhere.

After looking for hours it seems that the hoister does not support the case when a workspace-dependency cannot be symlinked in the root node_modules if a dependency with the same name is already going to be installed there. But installing dependencies in the workspace node_module folder does right now rely on being able to have the workspace linked in the root node_modules folder.

@danez
Copy link
Contributor

danez commented May 20, 2019

I created a PR that hopefully is going to fix this. #7289

@nicolo-ribaudo
Copy link

I was trying to enable workspaces in Babel and then found this issue. Is there anything I can do to help moving it forward?

@arcanis
Copy link
Member

arcanis commented Nov 18, 2019

I gave another look and it sounds good to merge if the rebase is still green. I will make a pass on a few other PRs tomorrow and release all that 🙂

@nicolo-ribaudo
Copy link

Thanks ❤️

@nicolo-ribaudo
Copy link

Thanks!

@arcanis
Copy link
Member

arcanis commented Nov 24, 2019

The 1.19.2 should be available - to upgrade inside the Babel repo, try running yarn policies set-version 1.19.2 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants