Circular Dependency bundling 'folktale/result' into library with Rollup.

[wking-io]

I am building a library that is using Result and when I am building a UMD bundle with Rollup I get Circular dependency errors

Steps to reproduce

Minimal example here with steps:

https://github.com/wking-io/folktale-bug

Expected behaviour

I expected this to be bundled successfully without any errors.

Observed behaviour

There was an error because rollup found a Circular Depenedency

(!) Circular dependency: ../../node_modules/folktale/result/result.js -> ../../node_modules/folktale/conversions/result-to-validation.js -> ../../node_modules/folktale/validation/validation.js -> ../../node_modules/folktale/conversions/validation-to-result.js-> ../../node_modules/folktale/result/result.js
(!) Circular dependency: ../../node_modules/folktale/result/result.js -> ../../node_modules/folktale/conversions/result-to-validation.js -> ../../node_modules/folktale/validation/validation.js -> ../../node_modules/folktale/conversions/validation-to-result.js-> commonjs-proxy:/Users/wking/sites/saladbar/node_modules/folktale/result/result.js -> ../../node_modules/folktale/result/result.js
(!) Circular dependency: ../../node_modules/folktale/result/result.js -> ../../node_modules/folktale/conversions/result-to-validation.js -> ../../node_modules/folktale/validation/validation.js -> ../../node_modules/folktale/conversions/validation-to-maybe.js -> ../../node_modules/folktale/maybe/maybe.js -> ../../node_modules/folktale/conversions/maybe-to-result.js -> ../../node_modules/folktale/result/result.js
(!) Circular dependency: ../../node_modules/folktale/validation/validation.js -> ../../node_modules/folktale/conversions/validation-to-maybe.js -> ../../node_modules/folktale/maybe/maybe.js -> ../../node_modules/folktale/conversions/maybe-to-validation.js -> ../../node_modules/folktale/validation/validation.js
(!) Circular dependency: ../../node_modules/folktale/validation/validation.js -> ../../node_modules/folktale/conversions/validation-to-maybe.js -> ../../node_modules/folktale/maybe/maybe.js -> ../../node_modules/folktale/conversions/maybe-to-validation.js -> commonjs-proxy:/Users/wking/sites/saladbar/node_modules/folktale/validation/validation.js -> ../../node_modules/folktale/validation/validation.js

Environment

(Describe the environment where the problem happens. This usually includes:

• macOS
• Node 8.9.4
• Folktale 2.1.0

[robotlolita]

Folktale uses circular dependencies, which both ES2015 and Node modules support. That's not an error.

I don't know if Rollup's support for importing CommonJS modules works with mutually recursive modules. If it doesn't (i.e.: if a bundle isn't generated, and those are not just warnings), then at this point Folktale doesn't support Rollup. You could first bundle Folktale with something like Browserify, then import that---but yeah, that'd be a pain. The next version of Folktale will use ES2015 modules, but it won't be released before June or July this year.

If Rollup does generate a bundle, but outputs those warnings, you can ignore the warnings for Folktale.

Note that the next version of Folktale, with ES2015 modules, will still use mutually recursive modules. It seems that Rollup emits warnings on mutually recursive ES2015 modules though, even though the specification says that you must support that use case, which is kind of weird to me. I don't know why Rollup would emit a warning in that case. Rollup may have a bug in how it handles module bindings, so it emits a warning; or it may think that circular dependencies are not desired, and emits a warning. Their website is not up right now so I can't look at their docs, but you might be able to find more information on why you're seeing these warnings there.

[Undistraction]

Note that this is potentially a bigger issue with Webpack 4's support of sideEffects: false. Packages with no side-effects (in the ES6 module sense), and (I think) no circular dependencies can declare themselves side-effect free by using this flag in their package.json, allowing Webpack to be much more aggressive in its tree-shaking (and potentially make huge savings on bundle size). I believe that Folktale's use of circular dependencies will preclude it, and possibly any lib that uses it from using this flag. I'm not 100% sure on this as info is a bit scarce at the moment, but I'll try and dig up some more information.

[robotlolita]

@Undistraction the idea of being able to add meta-data about runtime semantics that might not necessarily match the runtime semantics, then base optimisations off that gives me so many Common Lisp's optional typing optimisation vibes... It's not a good idea (what you get out of the compiler is unpredictable) :/

ES2015 modules' mutually recursive dependencies shouldn't affect tree-shaking, though. If it does, sounds like a bug in the library implementing the algorithm.

[Undistraction]

@robotlolita Yep. The whole thing sounds suspect to me and has the potential of nightmarishly hard to track-down bugs, but the savings in bundle size are hard to argue with. I haven't managed to track down a definitive answer but I'll post here if I do.

[newtriks]

Adding external: ['folktale/validation'], to my Rollup config fixed the issue I had similar to this. The solution is specific to validation (maybe related to #180) but easily tweaked to result I am sure...