Cyclic dependency warnings for type-only imports

[marijnh]

If you have a file a.ts that declares a type and depends on b.ts

import {b} from "./b"
export class Foo { b() { return b } }

And a file b.ts that uses a type from a.ts in a type signature:

import {Foo} from "./a"
export function b(foo: Foo) {}

Then TypeScript will, by default, completely omit the dependency on a.js from the b.js output, since it's only necessary during typechecking.

However for some reason when bundling such files with this plugin, I get a warning:

(!) Circular dependency: a.ts -> b.ts -> a.ts

Which I guess means that Rollup does consider b.js to depend on a.js.

Somehow rollup-plugin-typescript2 gets this right, though I haven't been able to figure out what it is doing differently. (And I can't get that plugin to omit reasonable .d.ts files, so I'd much prefer to move to this one.)

[wessberg]

Hi there,

The reason why you're seeing circular dependency warnings with this plugin, but not with other Typescript plugins, is that this plugin supports Emit-less types which means that this plugin can track and generate declarations from files that either only exports ambient declarations or from which only ambient declarations such as types are imported (such as in your example).

As you say, Typescript would remove the import (unless emitDecoratorMetadata is given in your tsconfig), that's also why with other plugins Rollup would never consider the file as being referenced by the source file, but this plugin is actively re-adding imports back into the code in a transformation step such that the files will be included in the Rollup graph. We need it later in the declaration bundling and tree-shaking phase.

So essentially, this is a conscious design choice.

I realize, however, that the circular dependency warnings feels a bit misplaced (even though they are technically correct) given that there's no potential consequences. On the other hand, should you decide in the future to start emitting metadata from your types, you'll want to avoid circular dependencies anyway.

Feel free to comment again if this didn't make sense or if you have any input.

// Frederik

[marijnh]

Thank you for the explanation. This also explains another failure I was seeing, where a superclass ended up after a class that extends it in the output (leading to broken output)—the file defining the superclass imported a type from the one that defines the subclass, and I guess rollup just considered them a cyclic dependency that it could order whichever way.

Do you know rollup-plugin-typescript2 solves the emit-less issue? (Assuming it has — I've been using it quite heavily for a while now and haven't had any issues.)

[wessberg]

rollup-plugin-typescript2 have no issues with emit-less types any longer, (see this issue for details, I'm also part of the discussion in that issue). It solves the problem by forcing declarations for the files that would otherwise be ignored by Rollup.

The main difference between these two plugins on that front, however, is that this plugin relies heavily on Rollups dependency graph in the declaration phase, especially for tree-shaking declarations by reference counting, and also for re-writing imports to reflect the chunk names generated by Rollup. rollup-plugin-typescript2 more or less just lets Typescript generate declarations without performing additional work.

It might be possible to not add the type-only imports back in, and instead enrich the data in the Rollup graph with the missing information while generating these declarations. That would remove the circular dependency warnings. Not sure how large of a task that would be, but I might be able to look into it at some point.

[wessberg]

@marijnh, This has been fixed in v1.1.74. That version gives you the best of both worlds - support for emit-less types, while at the same time not unnecessarily adding the ambient files to the Rollup graph leading to unwanted cyclic dependency warnings.

This also explains another failure I was seeing, where a superclass ended up after a class that extends it in the output (leading to broken output)—the file defining the superclass imported a type from the one that defines the subclass, and I guess rollup just considered them a cyclic dependency that it could order whichever way.

Yup, that has been fixed too.

[marijnh]

Thanks!