fix: enable compatibility with node16/nodenext module resolution

[boneskull]

Closes #1802

Description

TypeScript's node16 and nodenext module resolution algorithms attempt to adhere more closely to the behavior of the exports field. To that end, if an exports field is present in a package, the types field is ignored; instead, a types conditional export is expected.

Without this change, any consumer using one of these newer--officially recommended--module resolution algorithms will be unable to load any types.d.ts file from any package. We retain the types field because of backwards compatibility with the legacy node module resolution algorithm.

Read more about how I came to this conclusion

Should the types field be used at all?

It depends. While this file is not truly needed for type support if type declarations are shipped alongside each .js source file, it's awkward to export any types created via @typedef without it.

For instance, say we have a function foo(opts: FooOpts). foo lives in a.js, and FooOpts is a @typedef in b.js. The parameter opts is tagged as @param {import('./b.js').FooOpts} opts, as it should. Entry point index.js re-exports foo() from a.js. However, the definition for FooOpts will not be available to consumers--which will cause compilation errors. We must, then, re-export this type from the entry point. To avoid this, we have precious few tools at our disposal:

1. Create a reference of FooOpts in index.js via @typedef, which will export the FooOpts type. If FooOpts is generic, this becomes more of a headache, as we have to copy the generics from the original. This is a hazard, because the generics could become out-of-sync.
2. Create a .d.ts (it could also be a .ts, which we can assume will generate the associated .d.ts; this doesn't matter for our purposes) which re-exports FooOpts from b.js. This file will be the titular types.d.ts.

The question we need to ask, then, is "do we need to export any @typedefs?" If the answer is yes, then a types.d.ts makes sense.

I didn't actually check to see if types.d.ts is appropriate in all of these packages; I just fixed the problem I was encountering. For @endo/compartment-mapper, some work I'm doing will mean re-exporting some @typedefs, even if that's not happening currently. Furthermore, I ignored the packages with "types": null, since I don't understand what that's supposed to mean.

Testing Considerations

I did not add any tests, as the test needs to happen on the result of npm pack, which is more of a fixture than I wanted to build out. However, I am working on a tool which will (hopefully soon) automatically check for problems like this. Once that's ready, I can open a new PR to add the checks and you can see what you think.

[mhofman]

As a stop gap measure, this approach seems fine.

Should the types field be used at all?

I am still of the opinion that the answer is no, and that we should migrate away from these manual type definition files that shadow the runtime entrained definitions. We don't have to use JSDoc @typedefs to migrate away, just need to "runtime" re-export the types file from the entrypoint, but that does require generating type definition files in a prepublish build step, which these packages do not do yet.

[boneskull]

We don't have to use JSDoc @typedefs to migrate away, just need to "runtime" re-export the types file from the entrypoint, but that does require generating type definition files in a prepublish build step, which these packages do not do yet.

@mhofman I don't follow. Say we wanted to expose PackageNamingKit from types.js in @endo/component-mapper. How would we do that, exactly?

[mhofman]

@mhofman I don't follow. Say we wanted to expose PackageNamingKit from types.js in @endo/component-mapper. How would we do that, exactly?

• Remove the current types.d.ts which seems to only be a "types" copy of runtime exports present in index.js.
• Create an exports.js with only export {}; as content
• Create an exports.d.ts, with the following as content: export { PackageNamingKit } from './src/types.js';
• Add export * from './exports.js'; in index.js
• Remove any "types" field from package.json
• Add a prepublish step to generate .d.ts files from .js files like some other packages are already doing

[boneskull]

@mhofman I'll try that.

But it begs the question: why use a types.js anyway, when we could just rewrite it as types.ts using the convenient syntax? To me, a .js file containing only types seems like it should be a .ts file. Is there a practical/technical concern? Or a preference / philosophical one?

In this case, exports.js and export.d.ts would just become exports.ts, and the .d.ts would be generated in the prepack lifecycle script, then removed after.

[boneskull]

(btw, compartment-mapper already compiles declarations in its prepack script then blasts them in its postpack script)

[boneskull]

@mhofman By "I'll try that", I mean I will try that in another forthcoming PR. Care to approve?

FWIW, I just tried this in compartment-mapper, and it seems to work fine to:

• Remove any types anything from package.json
• Create a src/exports.ts which contains only export type * from './types.js'
• Add export * from './src/exports.js' in the root index.js

Mind you, within a node16/nodenext repo consuming @endo/compartment-mapper, TS finds no types whatsoever unless the .d.ts files have been generated. e.g. If I link compartment-mapper into my own project, I have to run its prepack script to have type information. I don't think this is unusual behavior--though it is awkward for my use case.

[mhofman]

In this case, exports.js and export.d.ts would just become exports.ts, and the .d.ts would be generated in the prepack lifecycle script, then removed after.

Because that requires the generation of the output not just for publish but also for authoring inside the repo, which I am against.

TS finds no types whatsoever unless the .d.ts files have been generated. e.g. If I link compartment-mapper into my own project, I have to run its prepack script to have type information. I don't think this is unusual behavior--though it is awkward for my use case.

That is exactly the behavior I find unacceptable. There should be no generation step needed when working within the repo (which effectively also uses links through yarn workspace)

[boneskull]

@mhofman

Because that requires the generation of the output not just for publish but also for authoring inside the repo, which I am against.

It doesn't.

To be clear, the changes to package.json and the addition of exports.ts does not change how @endo/compartment-mapper interfaces with the rest of the workspaces in the project

That is exactly the behavior I find unacceptable. There should be no generation step needed when working within the repo (which effectively also uses links through yarn workspace)

There isn't!

It's only when working outside of the repo. By my own project I mean "a package somewhere in the lavamoat monorepo which links @endo/compartment-mapper."

I'm confident this all works the way you think it should.

[mhofman]

if there is an export * from './src/export.js' in index.js that means an export.js must exist or else the package will fail to load at runtime. If that file is generated, it means a generation step is required. Note this is not a concern to make types work, but a concern to keep runtime imports of the package working when the package is imported by other packages in the same repo.

[boneskull]

@mhofman Ah, you're right. Anyway: the code in question doesn't exist. I am trying to get this PR merged, though. ;)

cc @naugtur @kriskowal

[naugtur]

improvement. no risks. let's merge