Make types importable in Deno

[bwalderman]

This change enables the devtools-protocol typings to be more easily imported in ESM-based environments like Deno.

A new esm directory has been generated alongside the existing types directory. esm contains a copy of the Typescript definitions that play nice with ES module importing. CommonJS code or TypeScript code targeting Node.js will continue to work as-is and can import the definitions from the /types directory. ESM code can import from /esm instead.

Motivation: A single file with no imports of its own like protocol.d.ts can already be imported in either an ESM or CommonJS environment without much trouble. The issue comes when trying to import a file that has further imports like protocol-mapping.d.ts. This file imports the protocol.d.ts module but was written for one environment and won't work in the other. Specifically, ESM code requires a path to a file to be imported including the file extension (e.g. ./protocol.d.ts). On the other hand TypeScript code targeting a Node.js environment requires no file extension (e.g. ./protocol). The current state of TS/JS tooling seems to require having two different files to support both kinds of environment. More info at https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html.

[TimvdLippe]

On the other hand TypeScript code targeting a Node.js environment requires no file extension (e.g. ./protocol).

I don't think this is strictly true, as we can append the .js extension in Node as well. When I try to import devtools-protocol with ESM and TypeScript, it appears to work:

import Protocol from 'devtools-protocol';

Protocol.Animation.AnimationType.CSSAnimation;

Could you let me know how I can reproduce the problem you are experiencing and how I can verify that the problem is fixed? My hunch is that, rather than appending .d.ts at the end of the import, I think we can add .js add the end of the import and that should work both in CommonJS and ESM. That would negate the need for duplicating all of our types, as a single type definition file can be used, can't it?

[bwalderman]

HI @TimvdLippe, yes the Typescript compiler does seem to allow a .js extension on imports. It's .ts that it explicitly disallows. There's an ongoing discussion about this decision and the implications for ESM support in this thread: https://github.com/microsoft/TypeScript/issues/37582.

The example you provided happens to work in either Node or Deno, but here's one that currently only works in Node. It imports types/protocol-mapping and uses it to make a type-safe sendCommand function:

import { ProtocolMapping } from 'devtools-protocol/types/protocol-mapping';

function sendCommand<C extends keyof ProtocolMapping.Commands>(method: C, ...params: ProtocolMapping.Commands[C]["paramsType"]): Promise<ProtocolMapping.Commands[C]["returnType"]> {
    return Promise.reject("todo");
}

sendCommand("Target.createTarget", { url: "https://bing.com" }); // Okay.
sendCommand("Target.createTarget", { foo: "bar" }); // Should fail type check.

There's an example gist here that you can download and npm install. If you open this example in VS Code or another IDE with Typescript language support, it should highlight the last line with a type error. This is because it was able to resolve the Protocol.Target.CreateTargetRequest type, and { foo: "bar" } does not fit this type. So far so good. Remove that line, and npm run start to run the example. It should compile and run the example. This is great; we have full IDE support, and our program runs once we fix the type error.

Here's a Deno version of the same example:

import { ProtocolMapping } from "https://cdn.skypack.dev/[email protected]/types/protocol-mapping.d.ts";

function sendCommand<C extends keyof ProtocolMapping.Commands>(method: C, ...params: ProtocolMapping.Commands[C]["paramsType"]): Promise<ProtocolMapping.Commands[C]["returnType"]> {
    return Promise.reject("todo");
}

sendCommand("Target.createTarget", { url: "https://bing.com" }); // Okay.
sendCommand("Target.createTarget", { foo: "bar" }); // Should fail type check.

Deno's ESM module resolution requires extensions on all imports, so protocol-mapping.d.ts can be imported successfully using the above code. But it fails to resolve import Protocol from './protocol' inside protocol-mapping.d.ts because that import is missing the .d.ts extension. So the IDE can't find any Protocol.* types and it fails to catch the type error on the last line. Try running this example with deno run main.ts, and it reports that it was unable to resolve the ./protocol module:

error: Import 'https://cdn.skypack.dev/-/[email protected]/dist=es2019,mode=raw/types/protocol' failed, not found.
    at https://cdn.skypack.dev/-/[email protected]/dist=es2019,mode=raw/types/protocol-mapping.d.ts:5:22

We could fix this for Deno by importing ./protocol.d.ts in protocol-mapping.d.ts but that would break existing Node users and vice versa. That's why I'm proposing providing two sets of files. As far as I can tell based on that Typescript thread above, protocol-mapping.d.ts can't currently be written in a way that it would "just work" in both environments.

[bwalderman]

Thought about this some more. A better idea might be to add the ProtocolMapping and ProtocolProxyApi namespaces to protocol.d.ts. So everything would exist in one flat file and consumers can just import what they need from "devtools-protocol". No need for duplicate files under different paths. Would this be acceptable?

[TimvdLippe]

Yes that sounds like a better idea to me. If we can avoid file duplication, that would be ideal. That said, ProtocolMapping and ProtocolProxyApi themselves depend on Protocol, so I don't think we can add a circular dependency for that. Instead, we could add an extra entrypoint that would be importable by both and that might work. @jackfranklin you have done some work in this area for Puppeteer, do you have any guidance for us?

[bwalderman]

Yeah, to avoid circular dependencies and to avoid the use of import statements, all of Protocol, ProtocolMapping, and ProtocolProxyApi would need to be defined in protocol.d.ts. We would want to keep the existing protocol-mapping.d.ts and protocol-proxy-api.d.ts files around for backwards compatibility but recommend that new code imports from the main protocol.d.ts file instead. So there would still be some duplication because the ProtocolMapping and ProtocolProxyApi namespaces would be defined in two files. But, with this approach, users in CommonJS and ESM environments don't need to import from different paths. And, if the TypeScript and Node folks eventually settle on a nice way to handle the import problem, then at that point we should be able to import ProtocolMapping and ProtocolProxyApi into protocol.d.ts instead of duplicating them, and we can make that fix transparently to end users.

[brendankenny]

commonJS vs. ESM is kind of a red herring because these aren't really ESM files, they're d.ts files. They have no emitted form and only exist at compile time. It would be nice if Deno allowed no file extension and/or tsc allowed .d.ts extensions on import type statements, since for both there would be no ambiguity, but maybe one or the other will get there before too long.

It does seem like it would be good to avoid package.json exports until https://github.com/microsoft/TypeScript/issues/46452 is figured out, and ideally we wouldn't make too many drastic changes until https://github.com/microsoft/TypeScript/issues/37582 makes progress.

• I assume --no-check works for the import {ProtocolMapping} from 'devtools-protocol/types/protocol-mapping.d.ts' case but it wouldn't be ideal.
• What about import maps? I don't think we want downstream projects to have to use these to work around this individually but it's worth considering if there's an elegant way to use them
• does @deno-types suffer from the same transitive import issue?

[bwalderman]

Thanks @brendankenny. Yeah, I guess this is not really about CommonJS/ESM but about incompatible ESM module resolution rules in different Typescript environments.

I assume --no-check works for the import {ProtocolMapping} from 'devtools-protocol/types/protocol-mapping.d.ts' case but it wouldn't be ideal.

Yeah, disabling type checking wouldn't be ideal. Might as well not even bother using devtools-protocol types.

What about import maps? I don't think we want downstream projects to have to use these to work around this individually but it's worth considering if there's an elegant way to use them

I'm still learning about this stuff so it would be great if someone more familiar with import maps could weight in. From what I see though, they don't encourage using import maps to enable extension-less imports: https://github.com/WICG/import-maps#extension-less-imports. They do provide a straightforward example, but it's for remapping a bare module specifier like lodash. The problematic module specifier we're dealing with is ./protocol which is URL-like. These are resolved relative to the import map's base URL and the import map belongs to the downstream app; not the module being imported. So downstream apps would need to have an import map entry that looks something like:

"./path/to/devtools-protocol/types/protocol": "./path/to/devtools-protocol/types/protocol.d.ts"

This is slightly cumbersome if you're importing devtools-protocol from the local file system, but I ran into even more trouble creating a working entry when importing from a CDN. I could only get it to work by using the full URLs that Deno tries to load at runtime and tacking on the extension:

{
  "imports": {
    "devtools-protocol-mapping": "https://cdn.skypack.dev/pin/[email protected]/mode=raw/types/protocol-mapping.d.ts",
    "https://cdn.skypack.dev/-/[email protected]/dist=es2020,mode=raw/types/protocol": "https://cdn.skypack.dev/-/[email protected]/dist=es2019,mode=raw/types/protocol.d.ts"
  }
}

This was the most elegant solution I could come up with. My sample file compiles and runs, and VS Code picks up types too. It's not too bad for a module like devtools-protocol that only has a single nested import that needs fixing. This works for devtools-protocol but would quickly become annoying if trying to import another Typescript-based module that had more than just 1 or 2 extensionless imports to fix.

does @deno-types suffer from the same transitive import issue?

Yes. This is meant to be used with JS file imports to point to separate TS declarations, but there's no JS file in our particular scenario, and I've confirmed that the same transitive import problem occurs.

As of right now, our best options seem to be:

1. The import map workaround: Provides full IDE and compile-time support without any changes to this repo, but users need to give the full URL to the .d.ts files several times. This can be annoying to update.
2. Provide all namespaces in a single file with no imports, keeping the separate files for existing Node users. This gives new users a more elegant solution for their projects but requires some source duplication in this repo.
3. Wait for Deno and/or Node to relax their module resolution rules.

I'm still leaning towards (2). Aside from the pros mentioned earlier, exporting all three namespaces from a single entry point seems more idiomatic in an ESM world and might be a good idea regardless of the Deno/Node import issue.

[bwalderman]

I've gone back over the issues and read the import maps explainer more closely. I've updated the PR to implement solution (2) above, exporting all namespace in a single file.

It looks like TypeScript is unlikely to budge on their stance for making file extensions optional, based on their responses to the linked issue and others like it. Similarly, Deno seems set on requiring file extensions in import paths.

While import maps can technically be used to "fix-up" problematic imports, this is not recommended and not the intended purpose of import maps which is to enable bare specifiers like "lodash" in the browser. Using import maps for fix-ups is more of a last resort for developers who are having trouble getting a module to work.

Are there any concerns with the current solution? I'd be happy to update the readme as part of this PR.

[TimvdLippe]

All right. I had another go at this and I think I understand the problem better now. I am not thrilled about copying all of our APIs into another file, that seems to not be great.

That said, based on https://deno.land/[email protected]/typescript/types#using-x-typescript-types-header I think we can somewhat fix this by adding empty .js files into the package. In other words, we add types/protocol-mapping.js to the NPM package, which is completely empty. Both TS and Deno should then appropriately load the .d.ts file, for example with https://cdn.skypack.dev/[email protected]/types/protocol-mapping.js

Sadly, I am not sure how I can test that it actually works. Do you know if adding the empty .js equivalents to the .d.ts will appropriately instruct Deno to resolve the types? Then importing with .js at the end of it should work in all environments.