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

In Yarn Workspaces, sub dependency .bin symlinks can override top level dependency .bin symlink #6300

Open
mlavina opened this issue Aug 22, 2018 · 4 comments
Assignees
Labels

Comments

@mlavina
Copy link

mlavina commented Aug 22, 2018

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When using yarn workspaces. Sub dependencies .bin symlinks override top level dependencies .bin symlinks.

NOTE this was fixed in non workspaces with #4937

If the current behavior is a bug, please provide the steps to reproduce.
Example: https://github.com/mlavina/yarn-symlink-bug

What is the expected behavior?

Please mention your node.js, yarn and operating system version.
Node: 8.9.1
Yarn: 1.9.4
OS: Windows 7

Tagging @anders-advisa and @rally25rs Since they worked on the similar bug mentioned above

@ghost ghost assigned torifat Aug 22, 2018
@ghost ghost added the triaged label Aug 22, 2018
@mlavina
Copy link
Author

mlavina commented Sep 18, 2018

@torifat
Was wondering if you had a chance to look at this?

@mlavina
Copy link
Author

mlavina commented Nov 6, 2018

@torifat any updates here?

@mlavina
Copy link
Author

mlavina commented Nov 6, 2018

Maybe @arcanis you can take a look it looks like torifat hasn't had a chance to look at a bunch of issues. I am glad to help with this issue and file a PR if i could get some guidance on where the issue might be and where to start.

tiffon added a commit to jaegertracing/jaeger-ui that referenced this issue Jan 7, 2019
Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>
tiffon added a commit to jaegertracing/jaeger-ui that referenced this issue Jan 9, 2019
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR #346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>
everett980 pushed a commit to everett980/jaeger-ui that referenced this issue Jan 16, 2019
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR jaegertracing#346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Everett Ross <reverett@uber.com>
@ndreckshage
Copy link

ndreckshage commented Mar 19, 2020

Created my own demo of this before coming across this issue: https://github.com/ndreckshage/yarn-workspace-bin-issue

In my example:

  • pkga lists pretter@1.18.2 as direct dep
  • pkgb has has a dep which pulls in prettier@1.19.1
  • yarn hoists 1.18.2 version to root node_modules (correct)
  • yarn hoists 1.19.1 version to root node_modules/.bin (incorrect)

This is still an issue in 1.22.4.

vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this issue Jul 5, 2021
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR jaegertracing#346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>

Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this issue Jul 5, 2021
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR jaegertracing#346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Everett Ross <reverett@uber.com>

Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants