esm extentions/code generation

[brudil]

Currently doing some initial poking around in our codebase to see if the time is right for a full move to esm; our TypeScript config currently just targets commonjs.

The requirement of file extensions for all imports (.js) is obviously going to be a sticking point for generated code.

I'm imaging this could be a option, as the easiest route. But also thinking this might be more down to ts-poet, rather than joist's codegen itself.

Any thoughts I'm all ears. Not a pressing issue, but something worth thinking about.

[stephenh]

Hey! Coincidental timing! I was just thinking of this over the weekend, as I've had about ~3 (failed) attempts to get Joist's test suite running on vitest, and after this 3rd attempt, I'm pretty much convinced that it's not working due to joist-orm getting dual-loaded as both a CJS module and ESM module, and so the various module-level consts/caching within joist-orm getting confusing. 😢

(Basically vitest is fundamentally ESM-first, and so doing things like "bootstrap the domain objects" as in ESM mode, but then Joist's test suite is still CommonJS, so when it goes to actually invoke the domain objects to do things like hooks, etc., they get ran in "CommonJS mode", so get a CJS duplicate of the joist-orm module/state..)

Granted, I was just trying to get Joist's own integration-tests onto vitest, so maybe a downstream project could use vitest with its entities, but I dunno, I'm guessing probably not.

But, anyway, yeah, seems like we need to at least have an ESM flag, in joist-config.json?, that outputs all the necessary .js suffixing "nonsense". :-) I say that kind of jokingly, but also seriously, like should we output .js extensions like we're supposed to, or allow *.ts extensions like I believe a lot of projects will probably end up using, and then deferring to a bundler to fix them up to *.js in the output?

I wonder what tsx does for extensions...like maybe I just need to accept *.js extensions for TS files as another lot in file. :-)

ts-poet

Good call on leaning into ts-poet for some sort of "one ts-poet config flag automatically flips all imports from no-extension to yes-extension"...hadn't thought of that!

Hopefully just the extension is most of it? Well, and deciding whether we want to keep shipping a CJS package, like dual-ship CJS & ESM artifacts for awhile... Probably so...

So hopefully it's just:

1. Get extensions output based on a config flag,
2. Update the build to produce dual CJS/ESM artifacts
3. 🤞 that our knex/pg dependencies work fine in ESM mode

Let me know if you'd like to hack around on any of this, would be really appreciated! It's on my todo list as well, honestly primarily to get our internal test suite moved over to vitest with the assumption it will be faster, but kinda on the back-burner at the moment.

[brudil]

Something worth adding here, I've played around with the experimental node loader hooks, as our app will need a replacement for (https://www.npmjs.com/package/tsconfig-paths) as this doesn't support rewriting native import statements, only require() - the API is only Stage 1 so will probably change, but I think it would also be possible to implement a hook for esm resolution/extentions. So we might get away with using TS moduleResolution: bundler option, without actually needing a second step/bundler, but rewriting within the hook.

(Our codebase uses decorators w/ metadata via type-graphql so we're stuck with actually using tsc rather than being able to use another tool to purely strip types.)

POC of a `tsconfig-paths`-like hook

// $ node --experimental-loader ./tsconfigpaths.js main.js

// tsconfigpaths.js
import { readFileSync } from "node:fs";

const aliases = JSON.parse(readFileSync('tsconfig.json', { encoding: 'utf-8' })).compilerOptions.paths;

export function resolve(specifier, context, nextResolve) {
    const alias = aliases[specifier];
    if (alias !== undefined) {
        return {
            ...context,
            shortCircuit: true,
            url: new URL(alias[0], import.meta.url).href.replace('src', 'dist')
        }
    }

    return nextResolve(specifier);
}

[stephenh]

I added an importExtension flag to ts-poet as a guess of what would be useful to drive conditional sometimes *.js / sometimes *.ts / sometimes extension-less output:

https://github.com/stephenh/ts-poet/pull/49/files

[brudil]

@stephenh 7 hours hence, and I've got there. Running in ESM, moved to Vitest and all passing.

Apologies hijacking this thread, but if you're interested I'll detail the journey (and a couple of traumatic bits along the way)

We've been a CJS tsc-built app, tsc for build has been a requirement due to using type-graphql which requires the emitDecoratorMetadata feature.

Testing has been via Jest - pointing to dist rather than source to avoid the 3x time cost of having ts-jest transforming things. As your can imagine this hurt DX, with IDE integration not fully understanding the mapping.

We also use some tsConfig paths mapping too, which during running has had to be handled via node -r ts-paths.js https://github.com/dividab/tsconfig-paths. Also configured within Jest's moduleNameMapper settings too.

I hadn't planned this work but the DX issues of testing were getting to me - wall time isn't everything. So I set out to restore ts-jest and start testing source again.

Then I some how came across that SWC had implemented emitDecoratorMetadata support - I always assumed this would be a nightmare to implement without essentially reimplementing tsc itself and we would be on tsc for build forever.

@swc/jest was near drop-in for ts-jest and got our testing suite time close to testing against dist again. A massive win for DX.

Obviously, if we're building for tests with swc, we might as well build with it too. Worked well. All in CJS.

With such a win behind me, the bravado was there. The albatross, ESM was next.

First thing was to tell SWC to emit esm.

"module": {
    "type": "es6"
  },

The idea was to output .mjs files, but this was quickly abandoned over setting "type": "module" in package.json.

I utilised the eslint-plugin-require-extensions which after an extraordinarily long run with --fix it had updated 2k files to use .js extentions and handling the now disallowed directory imports from './utils' -> from './utils/index.js'.

With that runs were working but tsconfig-paths was no longer serving it's purpose. As I mentioned comment above, Node's Module Hooks were the right place to handle this now.

A basic example of this looks like

node-import-hooks.js

import { register } from 'node:module';

register('./node-ts-paths-hook.js', import.meta.url);

node-ts-paths-hook.js

import tsConfig from './tsconfig.json' assert { type: 'json' };
import { fileURLToPath } from 'node:url';

export const resolve = (specifier, context, nextResolve) => {
  if (specifier in tsConfig.compilerOptions.paths) {
    const conf = tsConfig.compilerOptions.paths[specifier];

    return nextResolve(
      fileURLToPath(
        new URL(
          conf[0].replace('./src', './dist') + '/index.js',
          import.meta.url
        )
      ),
      context
    );
  }

  return nextResolve(specifier, context);
};

We could now run with node --import node-import-hooks.js and our ts paths were resolving.

There was a bit of fixing to do, couple of requires to update, bits the eslint --fix missed. But the app was running further than ever.

Then, hitting the point of this issue, the Joist's codegen output. The option in ts-poet looks great, but afaict it's not exposed within joist-codegen yet - if your open to exposing this in joist-config.json I'm happy to PR.

Without that yet being an option but wanting to continue to push forward I hacked together some additional checks and mapping within the import hook to handle the codegen files

this is nasty but for a POC it worked

  const url = new URL(specifier, context.parentURL);
  if (url.href.match(/(entities\/codegen|entities\/factories)\/([A-Z]+)$/i)) {
    return nextResolve(`${specifier}.js`, context);
  }

  if (
    context.parentURL?.match(
      /(entities\/codegen|entities\/factories)\/([A-Z]+)/i
    ) &&
    specifier.includes('entities')
  ) {
    return nextResolve(`${specifier}.js`, context);
  }

  if (context.parentURL?.endsWith('entities.js')) {
    return nextResolve(`${specifier}.js`, context);
  }

• For SWC at least, you need to ensure that you're importing types seperately via import type. We have a autofix eslint rule for this, but it had some false negatives so a little bit of work was needed to handle those. Enabling isolatedModules in tsconfig helped point these out.
• A bit more wrangingly - moving to lodash-es over lodash and we were running!

---

I attempted to now get our ESM world running within Jest but hit a couple of issues (jestjs/jest#11434) which gave the impetus to go further and attempt a move to vitest.

Due to the need for emitDecoratorMetadata, we couldn't use vitest default rollup/esbuild transformation pipeline, unplugin-swc came to the rescue, we're now transforming our test source with swc too. vite-tsconfig-paths also made the mapping for ts paths easier on that side too. Because vitest bundles the whole extension issues we needed the Node Import Hooks when running the app were not needed here.

vitest.config.js

import swc from 'unplugin-swc';
import { defineConfig } from 'vitest/config';
import tsconfigPaths from 'vite-tsconfig-paths';

export default defineConfig({
  test: {
    globals: true,
    root: './',
    globalSetup: ['./jest.integration.setup.js'],
    setupFiles: ['./src/external/jest/jest.env.setup.ts'],
  },
  plugins: [
    tsconfigPaths(),
    // This is required to build the test files with SWC
    swc.vite({
      swcrc: true,
    }),
  ],
});

A bit of updating our globalSetup (running pg and redis via test-containers for integration tests) and the env setup later, and we have vitest running. For us, that's when I hit the worst issue. Our tests were failing with a graphql error about multiple versions being used. We use rush/pnpm so I knew it wasn't a version issue. It was a CJS & ESM issue - the Dual package hazard https://nodejs.org/api/packages.html#dual-package-hazard.

GraphQL - I believe incorrectly - sets it's package.json "main" to an extentionless file. This causes issues. But a pnpm-patch for graphql to the rescue.

-  "main": "index",
+  "main": "index.js",
   "module": "index.mjs",

(I think this will be resolved in graphql@17 in the future)

And we were rolling. A couple of tests to update jest -> vi. It's done. It's possible, which on some last attempts I was starting to question if we would ever be able to move across.

Hope some of this was helpful!

[stephenh]

Great update @brudil ! Amazing to hear you got it working, albeit after a good bit of elbow grease. 😬 :-)

type-graphql which requires the emitDecoratorMetadata feature

Ah! That is an interesting wrinkle; we're a GQL codebase as well, but do the *.graphql SDL file -> codegen -> resolver approach, which has pros/cons.

SWC had implemented emitDecoratorMetadata support

Ah wow, that's a huge win. We've been using tsx for all the things recently (no good jest integration for it though 🤔 so still using ts-jest w/swc 🤷 ), but yeah haven't needed emitDecoratorMetadata.

With such a win behind me, the bravado was there

:-D

tsconfig-paths was no longer serving it's purpose.

I only just found it ~few weeks ago, and haven't really used it yet, but afaict Node has this capability now:

https://nodejs.org/api/packages.html#subpath-imports

That might be able to replace the custom resolve hook?

Still pretty neat how short & sweet that hook is to do though.

For us, that's when I hit the worst issue.

Lol, I thought we were like 3 "worst issues" into this already. :-D

sets it's package.json "main" to an extentionless file. This causes issues.

Wow. I mean, I know about the dual-package hazard, but that is super-subtle. Amazing find!

The option in ts-poet looks great, but afaict it's not exposed
within joist-codegen yet - if your open to exposing this in
joist-config.json I'm happy to PR.

Yep! Open to either an explicit flag like fileExtension: .js or esm: true or even auto-scanning the project's ./package.json for a type: module (maybe that is too cute?).

I get the impression that file extensions like "should imports use .js or .ts" is the subject of a lot of debate/hand-wringing, so I anticipate that it will probably need to be a configurable option at some point.

Hope some of this was helpful!

This was amazing! Great write up! Hopefully it also provided some catharsis. :-)

And, yep, if you'd like to PR a way to set the ts-poet flag appropriately, that would be great.

We also have this new joist-orm GitHub organization now, so if you wanted, it might be slick to do like a joist-sample-esm project that is like ~2 test entities + the misc setup to show Joist + ESM + vitest all working together?

It probably doesn't need to cover the emitDecoratorMetadata / type-graphql side of your setup, but I think having an ESM-dedicated sample would be great for testing releases / regressions / etc.

(In theory it'd be best for an esm-sample/esm-test-harness project to be hosted directly in the joist-orm repo at like packages/tests/esm or something, but I've kinda assumed get a sub-package to be ESM while the rest of the packages stay CJS (for now) would be a nightmare to setup / not a faithful reproduction of a true standalone/ESM Joist-consuming project, but 🤷 making a few assumptions.)

(I'm also happy to piece together the joist-sample-esm at some point if you'd like to focus on the just the ts-poet flag that your project needs.)

Thanks!

[brudil]

Closing this as #1029 landed "esm": true in joist-config.json, #1029 landed generation which is valid against tsconfig's verbatimModuleSyntax

[brudil]

I only just found it ~few weeks ago, and haven't really used it yet, but afaict Node has this capability now:

https://nodejs.org/api/packages.html#subpath-imports

That might be able to replace the custom resolve hook?

@stephenh Just wanted to chime in and say thanks for pointing me to this! Was completely unaware of this feature, tsconfig paths completely removed now. Node import hook no more!

[stephenh]

tsconfig paths completely removed now

Ah hey that's great! We'll plan on moving over to it too then, at some point...

[brudil]

TS's support is really nice as it knows your rootDir and outDir so the package.json imports obviously point to dist. Alas other the story for other tooling isn't great yet. Vitest's story isn't great yet, had to add in this big ol resolve alias map

"#zest/flag": new URL('./src/zest/flag/index.js', import.meta.url).pathname,
"#zest/cache": new URL('./src/zest/cache/index.js', import.meta.url).pathname,
"#zest/mail":  new URL('./src/zest/mailer/index.js', import.meta.url).pathname,
"#zest/task": new URL('./src/zest/task/index.js', import.meta.url).pathname,
"#zest/signal": new URL('./src/zest/signal/index.js', import.meta.url).pathname,
"#zest/filter": new URL('./src/zest/filter/index.js', import.meta.url).pathname,
"#zest/log": new URL('./src/zest/log/index.js', import.meta.url).pathname,
"#zest/context":  new URL('./src/zest/context/index.js', import.meta.url).pathname,
"#zest/gql":  new URL('./src/zest/gql/index.js', import.meta.url).pathname,
"#zest/action": new URL('./src/zest/action/index.js', import.meta.url).pathname,

I assume some native remapping support will soon be added to tooling which deal with source other output as this feature catches on.

Edit: considering so many tools already read and use tsconfig's paths, I wonder if they could/should utilise the ourDir and rootDir.