hfour/wsrun

optionally disable cycle detection

gavar opened this issue · 9 comments

gavar commented

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 commented

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 commented

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 commented

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 commented

(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 commented

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 commented

That sounds like a really nice idea!

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 commented

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)

can we disable this somehow ?