jssm does not work with TypeScript's Node16/NodeNext/Bundler moduleResolution

[tylerbutler]

Describe the bug

jssm doesn't export types correctly for Node16+'s ESM module resolution. This means that using jssm with a project using moduleResolution: Node16 or NodeNext, which is what TypeScript recommends for Node projects, fails to compile because of missing types.

To Reproduce

Here's a minimal repro project: https://github.com/tylerbutler/jssm-node16

npm run build in that project will invoke tsc and produces this error:

src/index.ts:1:44 - error TS7016: Could not find a declaration file for module 'jssm'. '/home/tylerbu/code/jssm-node16/node_modules/jssm/dist/jssm.es6.mjs' implicitly has an 'any' type.
  There are types at '/home/tylerbu/code/jssm-node16/node_modules/jssm/jssm.d.ts', but this result could not be resolved when respecting package.json "exports". The 'jssm' library may need to update its package.json or typings.

1 import { from as createStateMachine } from "jssm";

See https://arethetypeswrong.github.io/?p=jssm%405.98.2 and you'll see that jssm doesn't work in either of the modern TypeScript moduleResolution settings because the types can't be resolved. arethetypeswrong is a great tool to investigate and validate your export map.

Possible fix

Create a rolled-up declaration for ESM and CJS using rollup-plugin-dts. This seems to work well in my testing and is the smallest fix I know of. I prototyped those changes and tested them on the repro project listed above and compilation worked as expected for both CJS and ESM. Prototype changes can be seen here: https://github.com/StoneCypher/jssm/compare/main...tylerbutler:jssm:types-fixes-2

I didn't test the dev experience when looking up types. Now that there's a single type rollup getting back to "sourcefiles" might not work as it did.

Options that are tempting but don't work

You might be tempted to make the exports map look like this, and use the same declaration files for all exports. I.e.

"exports": {
  "require": {
    "types": "./jssm.d.ts",
    "default": "./dist/jssm.es5.cjs"
  },
  "import": {
    "types": "./jssm.d.ts",
    "default": "./dist/jssm.es6.mjs"
  },
  "default": {
    "types": "./jssm.d.ts",
    "default": "./dist/jssm.es5.cjs"
  },
  "browser": "dist/jssm.es5.iife.cjs"
},

This setup creates the following problems:

• When importing in a CJS TypeScript project, the types are resolved as ESM but should be CJS: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md
• When importing in an ESM TypeScript project, the types are not resolved properly because the import statements in the declarations don't include file extensions: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/InternalResolutionError.md

Expected behavior

jssm should work in modern TypeScript projects.

Other notes

When I investigated this, I noticed that the tsconfig used by jssm still has moduleResolution: Node (now known as Node10) and module: ES2015, which is known to produce ESM that won't work in Node. Likely you're not affected by that since you're using Rollup.

You could consider moduleResolution: Bundler as well, but Node16 will guarantee any ESM code generated by TypeScript will work in Node16. (This isn't an option right now since the bundler option was added in TS 5.0, so you'd have to upgrade TS too.)

[StoneCypher]

I use JSSM with nodenext every day and it seems to export types just fine for me

JSSM does not get its esm from typescript; it gets it from rollup.

[StoneCypher]

i notice that this PR also makes extensive changes to the project which have nothing to do with types, such as renaming every file in the project in a way that the typescript manual expressly says not to do

[StoneCypher]

this also seems to permanently add a CLI for a tool that this library doesn't use, and has incorrectly modified the testing coverage to add the themes for the documentation extraction

there's a whole lot of stuff in this pr that didn't go mentioned

i'm not comfortable editing the entire source tree by filename in a way the manual says is wrong. can you help me understand why you tried to make a change like this? i'm already off in a different pr evaluating that for months to see if it's actually valid. it's very surprising to see someone try to sneak that in without mentioning it.

[StoneCypher]

it may be that this is the underlying issue. that was already recommended to me by @mhsdev a few months ago for similar-ish reasons.

if so a repair is a relatively straightforward package.json change

i will try this later today and see if it helps

[tylerbutler]

I use JSSM with nodenext every day and it seems to export types just fine for me

Here's a minimal repro project: https://github.com/tylerbutler/jssm-node16

npm run build in that project will invoke tsc and produces this error:

src/index.ts:1:44 - error TS7016: Could not find a declaration file for module 'jssm'. '/home/tylerbu/code/jssm-node16/node_modules/jssm/dist/jssm.es6.mjs' implicitly has an 'any' type.
  There are types at '/home/tylerbu/code/jssm-node16/node_modules/jssm/jssm.d.ts', but this result could not be resolved when respecting package.json "exports". The 'jssm' library may need to update its package.json or typings.

1 import { from as createStateMachine } from "jssm";

i notice that this PR also makes extensive changes to the project which have nothing to do with types, such as renaming every file in the project in a way that the typescript manual expressly says not to do

Sorry, I am sure there are better ways to address this issue than what I did to work around it. I'm not very familiar with rollup or the way jssm is built and released. That's one reason I didn't open a PR, just this issue.

i'm not comfortable editing the entire source tree by filename in a way the manual says is wrong. can you help me understand why you tried to make a change like this? i'm already off in a different pr evaluating that for months to see if it's actually valid. it's very surprising to see someone try to sneak that in without mentioning it.

Are you referring to the changes to add file extensions to the imports? E.g.

import { JssmError } from './jssm_error.js';

Those may not be necessary. I thought they were originally but maybe all that's needed is a types file and exports entry for each module kind. I vaguely remember trying to generate additional types and updating the export map but was out of my depth with rollup.

That said, my understanding is that at runtime, Node requires extensions - but maybe that's already addressed by by the rollup config.

[StoneCypher]

Here's a minimal repro project: https://github.com/tylerbutler/jssm-node16

I appreciate the dialogue on the topic. I will do my best to fix this. Having a repro helps a whole lot.

 

I'm not very familiar with rollup or the way jssm is built and released. That's one reason I didn't open a PR, just this issue.

Understood.

 

Are you referring to the changes to add file extensions to the imports? E.g.

import { JssmError } from './jssm_error.js';

Yes. This is a traditional torch fight in the typescript community.

For what it's worth, I want the thing you did to be correct, and I hold this as a grievance against the TS team.

 

I thought they were originally but maybe all that's needed is a types file and exports entry for each module kind.

Repro in hand, we will find out.

 

That said, my understanding is that at runtime, Node requires extensions - but maybe that's already addressed by by the rollup config.

Typescript provides extensions in its compile-downs. Rollup has that ability but said ability is not in use in this project.

You can verify the process step by step in the build/ directory, which contains all major residues along the way, if you're curious.

[StoneCypher]

I am happy to open a PR, but I am having trouble with the tests, and I may have missed some steps in my local builds or something.

I would like to learn more about this. There should be no steps; it should be enough to write npm install && npm run build.

[tylerbutler]

@StoneCypher I updated the description in this issue with a lot more detail, including a potential small fix using just the rollup config.