Deno: Emit for imported ts files is sometimes skipped

[lucacasonato]

For some reason the JS for https://cdn.dreg.dev/polyfill/node/process.ts is not emitted when imported from https://cdn.dreg.dev/package/[email protected] imported from deno cache https://cdn.dreg.dev/package/@rollup/[email protected]/dist/es/index.js.

$ rm -rf $DENO_DIR
$ deno cache https://cdn.dreg.dev/package/@rollup/[email protected]/dist/es/index.js
(snip)
Check https://cdn.dreg.dev/package/@rollup/[email protected]/dist/es/index.js
warning: Compiled module not found "https://cdn.dreg.dev/polyfill/node/process.ts"
  From: https://cdn.dreg.dev/package/[email protected]
    If the source module contains only types, use `import type` and `export type` to import it instead.
error: Uncaught SyntaxError: The requested module '/polyfill/node/process.ts' does not provide an export named 'default'

If I manually force an emit with deno cache it works.

$ rm -rf $DENO_DIR
$ deno cache https://cdn.dreg.dev/polyfill/node/process.ts
(snip)
Check https://cdn.dreg.dev/polyfill/node/process.ts
$ deno cache https://cdn.dreg.dev/package/@rollup/[email protected]/dist/es/index.js
(snip)
Check https://cdn.dreg.dev/package/@rollup/[email protected]/dist/es/index.js

cc @kitsonk

[kitsonk]

So this is caused by coupling to some unintentional (and undesired) behaviour that has been removed in master.

In the new infrastructure, when there are types available, tsc only sees the type files. This avoids the host of issues we have had with JavaScript suddenly being type checked, plus helps ensure that the type checking is "accurate".

https://cdn.dreg.dev/package/[email protected] serves up a X-TypeScript-Types header that resolves to https://cdn.dreg.dev/package/@types/[email protected] which contains:

export * from "/package/@types/[email protected]/index.d.ts";
export { default } from "/package/@types/[email protected]/index.d.ts";

So https://cdn.dreg.dev/polyfill/node/process.ts is a dependency of only the JavaScript file, and not the Dts file. This means that tsc never evaluates https://cdn.dreg.dev/polyfill/node/process.ts, and therefore never emits it.

A simple fix is to modify the Dts file to:

export * from "/package/@types/[email protected]/index.d.ts";
export { default } from "/package/@types/[email protected]/index.d.ts";
import "/polyfill/node/process.ts";

So that the JavaScript and the Dts files share the same dependencies.

I don't really think this is a breaking change (or technically a bug) because it relied on unintentional (and subjectively incorrect) behaviour. If we stopped using the emit from tsc during a check (which we hope to do) this problem would go away, but you could argue that the "check" is incorrect at the moment with tsc. It should only evaluate the Dts file(s) and totally ignore the JavaScript all together (which it what it does now).

The only suggestion I have is that we could iterate through the graph after we process the emit and warn people that files we expected to be emitted were not emitted and that they may not have accurate types. Something like:

warning: A module was unexpectedly not emitted: "https://cdn.dreg.dev/polyfill/node/process.ts"
  It was imported by "https://cdn.dreg.dev/package/[email protected]" but not imported by its type definition at "https://cdn.dreg.dev/package/@types/[email protected]"

[kitsonk]

Just for clarity, this only impacts code that meets the following conditions:

• The source module is a JavaScript module that imports directly or indirectly something that is "emittable" (TypeScript, JSX, TSX)
• That JavaScript modules has either a X-TypeScript-Types or a /// <reference types="" /> pointing to a Dts file.
• That Dts file does not import directly or indirectly that "emittable" module.

The reason why the breakage is desirable is that if you are attesting that the Dts files represents the types of the module, it also needs to accurately reflect the dependencies of that module to ensure that the type checking is accurate. If all of the dependencies are unchecked JavaScript, it is fair to assume the Dts file represents all of the types, but if those dependencies are type checked, the Dts file needs to include them as dependencies, so they can be considered when type checking the whole programme.

[kitsonk]

I have been thinking about this... I am thinking the right thing to do is fix it, but I don't know if it will really fix what people are trying to do here. Clearly we need to give stronger guidance on how to properly leverage JavaScript with types and TypeScript under Deno. Hopefully #8139 will help with some of that, and I think we might want to print out some warnings where people have different "runtime" dependencies then they have for their "type" dependencies, because it means that there could be potentially gaps in the type checking.

But I think I know what we need to do to fix this, plus a couple other scenarios I believe that are occuring.

[jsejcksn]

I'm seeing this for modules in the deno.land/std library:

warning: Compiled module not found "https://deno.land/[email protected]/fs/move.ts"

[kitsonk]

@jsejcksn need more information than that please.

[jsejcksn]

@kitsonk Not sure how to reproduce—just hacking on something right now (haven't been using version control). Using the method above fixed it (deno cache https://deno.land/[email protected]/fs/move.ts).

[kitsonk]

Likely it is #8309 then, which has a fix open for it.