Building with rollup ends with circular dependency warnings

[vivekkj123]

Hi,
On building this package with rollup, I got errors below,

+ rollup -c

src/polyfill.ts → dist/polyfill.js, dist/polyfill.mjs...                                              
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/polyfill.js, dist/polyfill.mjs in 9.9s

src/polyfill.ts → dist/polyfill.min.js...                                                             
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/polyfill.min.js in 11.1s

src/polyfill.ts → dist/polyfill.es6.js, dist/polyfill.es6.mjs...                                      
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/polyfill.es6.js, dist/polyfill.es6.mjs in 7.6s

src/polyfill.ts → dist/polyfill.es6.min.js...                                                         
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/polyfill.es6.min.js in 10.6s

src/polyfill.ts → dist/polyfill.es2018.js, dist/polyfill.es2018.mjs...                                
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/polyfill.es2018.js, dist/polyfill.es2018.mjs in 5.3s

src/polyfill.ts → dist/polyfill.es2018.min.js...                                                      
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/polyfill.es2018.min.js in 6.9s

src/ponyfill.ts → dist/ponyfill.js, dist/ponyfill.mjs...                                              
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/ponyfill.js, dist/ponyfill.mjs in 4.2s

src/ponyfill.ts → dist/ponyfill.es6.js, dist/ponyfill.es6.mjs...                                      
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/ponyfill.es6.js, dist/ponyfill.es6.mjs in 3.8s

src/ponyfill.ts → dist/ponyfill.es2018.js, dist/ponyfill.es2018.mjs...
(!) Circular dependencies
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream/generic-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/readable-stream.ts
src/lib/readable-stream.ts -> src/lib/readable-stream/async-iterator.ts -> src/lib/readable-stream/default-reader.ts -> src/lib/validators/readable-stream.ts -> src/lib/readable-stream.ts
...and 7 more
created dist/ponyfill.es2018.js, dist/ponyfill.es2018.mjs in 3.5s

Any fixes?

[MattiasBuelens]

These warnings occur because e.g. ReadableStreamReaderGenericInitialize in generic-reader.ts uses ReadableStream from readable-stream.ts, but that file also (indirectly) imports generic-reader.ts. This is fine because:

• ReadableStreamReaderGenericInitialize only uses the type of ReadableStream, it doesn't actually need to call that constructor.
• Even if it did call it, it's still fine as long as it doesn't do it while the module is being loaded.

They don't cause a miscompilation, and the resulting bundle still works fine (as evidenced by the CI passing all the tests).

Now, I do agree that it's better if we can avoid these warnings. Previously that wasn't possible, but nowadays TypeScript has type-only imports (import type { ... } from '...') which I think can help Rollup better understand that there isn't actually a circular dependency. I'll see if that would help.

[MattiasBuelens]

Ah, there are some other non-type-only imports too. 😅

• generic-reader.ts imports ReadableStreamCancel from readable-stream.ts
• default-reader.ts imports IsReadableStreamLocked from readable-stream.ts
• validators/readable-stream.ts imports IsReadableStream from readable-stream.ts

I guess I should move some of these functions out of readable-stream.ts and into a separate module instead to "break" these cycles. But since it's more of an internal stylistic issue, it's not on the top of my to-do list. I'll get around to it at some point (probably if/when I need to rewrite/restructure that code), but not right now.