hfour / wsrun

Command runner for Yarn workspaces. Dependency aware.
MIT License
512 stars 31 forks source link

wsrun doesn't mimick yarn workspace resolution #69

Closed VPagani closed 5 years ago

VPagani commented 5 years ago

I don´t know if that is intended but having @babel/core as a devDependency throws this:

ERROR: Dependency cycle detected:
   @babel/core <- debug <- @babel/core

wsrun is going to far on this because debug's devDependency shouldn't be considered in the cycle detection, should it?

What should I do?

spion commented 5 years ago

I'm not sure I understand - why are both babel-core and debug in the same workspace?

VPagani commented 5 years ago

@babel/core is a devDependency in one workspace of my project and debug is a dependency of @babel/core.

spion commented 5 years ago

It should not matter as long as @babel/core and debug are installed in node_modules and not workspace packages. devDependencies or dependencies that are not in the workspace are ignored.

Are you by any chance including your node_modules in your workspace glob? That would be the only reason I can think of why that would happen...

VPagani commented 5 years ago

Very strange, that's not the case. I ran yarn workspaces info | grep babel just to make sure I'm not that stupid. Is there any way to debug this dependency resolution?

VPagani commented 5 years ago

I did some digging on wsrun code. Put some console.log to check some values along the way. It turns out the glob resolution at https://github.com/hfour/wsrun/blob/master/src/workspace.ts#L29 is not mimicking the way yarn does.

filesAndDirs ends up with every file found in node_modules inside any workspace directory. Considering that Yarn for some reason doesn't hoist devDependencies to root node_modules, any workspace in my project that has @babel/core as a devDependency will produce a dependency cycle.

In comparison, yarn ignores any path that have node_modules https://github.com/yarnpkg/yarn/blob/v1.18.0/src/config.js#L811

Adding console.log(filesAndDirs.filter(path => path.includes("babel"))) at src/workspace.ts:30

[ '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/caching.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/config-chain.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/config-descriptors.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files/configuration.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files/index-browser.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files/index.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files/package.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files/plugins.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files/types.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/files/utils.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/full.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/helpers',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/helpers/config-api.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/helpers/environment.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/index.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/item.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/partial.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/pattern-to-regex.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/plugin.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/util.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/validation',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/validation/option-assertions.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/validation/options.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/validation/plugins.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/config/validation/removed.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/index.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/parse.js',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/tools',
  '[Sistemas]/[telefone]/gcphone_src/node_modules/@babel/core/lib/tools/build-external-helpers.js',
  ... 1382 more items ]
VPagani commented 5 years ago

Changing this line in src/workspace.ts:29 solved my problem, but I don't know if it's enough to mimick yarn workspace resolution:

let filesAndDirs = flatMap(globs, g => glob.sync(g).filter(path => !path.includes("node_modules")))
spion commented 5 years ago

What do your workspaces glob(s) look like?

VPagani commented 5 years ago
"workspaces": [
    "\\[Core\\]/**",
    "\\[Sistemas\\]/**",
 ]
spion commented 5 years ago

PR to add the ignore functionality welcome - I looked into how yarn does it and i can't figure out where it gets the ignore patterns from.

VPagani commented 5 years ago

Well, I looked deep into yarn code, it turns out to be just like:

workspaceGlob + "/node_modules/**/+(package.json|yarn.json)"

Maybe Yarn did a bit of overengineering on this for something simple 🤔

VPagani commented 5 years ago

Hey @spion, have you checked my pull request? Is there anything I need to do to improve it?