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

optionally disable cycle detection #65

Open
gavar opened this issue Aug 22, 2019 · 9 comments
Open

optionally disable cycle detection #65

gavar opened this issue Aug 22, 2019 · 9 comments

Comments

@gavar
Copy link

gavar commented Aug 22, 2019

Please provide option to disable cycle detection.

I'm using yarn workspaces in two projects written in typescript where they reference each other.
It works okay if I use ts-node to:

  • first make an in-memory compilation of the code
  • and only afterwards build a JS libraries using rollup
    It doesn't produce a dead loop, but wsrun throws an error even why I don't specify --recursive option.

Could be done by wrapping this block with new option, like --no-cycle-detection

let cycle = runner.detectCycles()

Please let me know if you want me to do a PR.

@spion
Copy link
Collaborator

spion commented Aug 22, 2019

Just removing that line won't be enough, as the rest of the program assumes there are no cycles. Its likely that the final RunGraph promise will never resolve

@gavar
Copy link
Author

gavar commented Aug 22, 2019

Makes sense. I could try to add to check if the package was already visited in a RunGraph

this.resultMap.set(pkg, ResultSpecialValues.Pending)

if (this.resultMap.has(pkg))
  return Bromise.resolve(this.resultMap.get(pkg));

@spion
Copy link
Collaborator

spion commented Aug 22, 2019

Line 156 will only run after all deps resolve, which will only resolve after their own deps resolve, thereby reaching the cycle. Whatever flag is set has to happen before the dependencies line (153), and hopefully that can be used afterwards as a filter in 153 i.e. allDeps(p).filter(d => !this.alreadyInitiated(d))

@spion
Copy link
Collaborator

spion commented Aug 22, 2019

(And unfortunately even then, its not enough to just set the flag, because that way the rungraph will only wait for a dependency for the first time. Instead, an entirely different dependency waiting strategy will be necessary)

@gavar
Copy link
Author

gavar commented Aug 25, 2019

solved it by --package option, luckily my packages are scoped so I have script
"wsrun": "wsrun -p @wrench/*"

What do you think of adding .pkgConf("wsrun") to yarn configuration so it could be possible to configure package filtering for all commands by default?

wsrun/src/index.ts

Lines 13 to 15 in 9d2a2a1

let yargsParser = yargs
.env('WSRUN')
.wrap(yargs.terminalWidth() - 1)

@spion
Copy link
Collaborator

spion commented Aug 25, 2019

That sounds like a really nice idea!

@StevenLangbroek
Copy link

Hey folks! I'm running into this issue as well. Building sequentially should work, particularly because one of the dependencies is just a devDependency, not a direct dependency. Is there anything we can do to help?

@spion
Copy link
Collaborator

spion commented Jul 27, 2020

The only idea that could work is to modify the dependency graph before passing it to the rest of the program (that would probably happen before cycle detection). For that we need likely the following:

  • describe a reasonable strategy for deciding when to accept cycle break-up (e.g. if a cycle is detected, remove the first devDependency in the cycle chain? All of them? What if multiple cycles are detected? Do we find the minimum number of links we can remove to break up all of them?)
  • implement a pre-processing function that modifies the links in the dependency tree before passing it to the original cycle detection and the rest of the program
    • alternatively we could run this as an alternative to cycle detection
  • write tests for this
  • come up with a flag for this (Not sure if we want it to be on by default)

@sibelius
Copy link

can we disable this somehow ?

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

No branches or pull requests

4 participants