microsoft / tslib

Runtime library for TypeScript helpers.
BSD Zero Clause License
1.25k stars 126 forks source link

tslib should follow standards for ESM/CJS detection #173

Open johncrim opened 2 years ago

johncrim commented 2 years ago

tslib's package.json has a few issues RE detecting whether it's CJS or ESM. Due to lack of standards support, it has to be special-cased in a number of places, eg jest-preset-angular. Workaround like this are needed in any projects using jest, typescript, and tslib.

The following changes to the package.json would fix things - happy to provide a PR if it will be considered:

  1. There is no "type": "module" field, which tells node code that .js files support ESM
  2. The ESM tslib (currently "tslib.es6.js") should have an .mjs file extension
  3. The CJS tslib (currently `"tslib.js") should have a .cjs file extension.

To be clear, just the type field would help, but I think it would be preferable to complete all 3 at once.

johncrim commented 2 years ago

Node has documented their logic for determining whether a module is CJS or ESM. This appears to be what other other projects, eg jest, are using to determine what type of syntax to expect when loading js files.

The node logic doesn't seem to address cases where both CJS and ESM files are published in a package, which seems like a major oversight. However, it should be possible to satisfy both while making no effective changes to tslib's package.json for clients depending on the current fields.

johncrim commented 2 years ago

@ahnpnl - I thought you might want to chime in on this issue, since you've done some work to special-case handling of tslib.

ahnpnl commented 2 years ago

What I experienced was: If using esbuild or swc to compile tslib.es6.js, Jest cannot process the compiled output. However, if I use TypeScript API to compile tslib.es6.js, Jest is happy with the output.

Possibly related to https://github.com/microsoft/tslib/issues/161#issuecomment-1032886001

wmertens commented 2 years ago

So are there any workarounds right now? I'm an indirect consumer of tslib, does anybody have a working npm package I can enforce using instead?

wmertens commented 2 years ago

I tried changing tslib.js so it exports a plain object with the functions in there including __decorate, and Jest still doesn't work, same error 😓

wmertens commented 2 years ago

as a workaround I made https://github.com/StratoKit/tslib

johncrim commented 2 years ago

Hi @wmertens - there are 2 fixes I've used:

  1. Explicitly link tslib.es6.js in your jest.config.js moduleNameMapper section, eg:
 ...
  moduleNameMapper: {
    tslib: 'tslib/tslib.es6.js',
  }
  ...

This is hard-coded in jest-preset-angular b/c the ability to resolve tslib ES6 in jest is broken (thus this issue).

  1. Create a copy of tslib.es6.js and rename it to tslib.mjs - jest (and node) always treat .mjs files as ESM. Then put it in a place, or update webpack config, so that it's linked instead of node_modules/tslib.
wmertens commented 2 years ago

@johncrim when I try option 1. it complains that node_modules isn't transpiled, and I'd prefer to keep it that way, unless there's a way to tell jest to only transpile tslib?

Option 2 is a variation of the package changes I made, so basically the same maintenance burden.

jahorton commented 1 year ago

What I experienced was: If using esbuild or swc to compile tslib.es6.js, Jest cannot process the compiled output. However, if I use TypeScript API to compile tslib.es6.js, Jest is happy with the output.

Possibly related to #161 (comment)

So are there any workarounds right now? I'm an indirect consumer of tslib, does anybody have a working npm package I can enforce using instead?

In case it matters to anyone, I've also posted most of what's written below to https://github.com/evanw/esbuild/issues/2296, as that issue is also quite related in my view.

My main project uses just tsc and esbuild, with no further tooling. (So, no Jest.) We recently found a way forward in ES5:

It's probably a bit hacky, but this relies on two details:

  1. esbuild version 0.15.16 and above now support alias options in their build configuration parameters. With this, we can alias tslib imports to a custom package that wraps the ES5-based tslib.js file found in the tslib distribution. esbuild can't handle the object-destructuring modules/index.js syntax in ES5 mode, as it requires ES6 - and that's what esbuild picks up when doing module-resolution.
  2. To ensure that we don't reproduce the same problem, the wrapping package's tsconfig.json uses "Node" module-resolution mode, not "Node16" or higher - we wish to ignore import maps. This instead connects us to tslib's package.json main entry, which is the CommonJS import leading to the pure-ES5 tslib.js, which esbuild can handle.

On second thought, it may be possible to add a resolution-phase plugin to do the redirect - the issue is about which tslib script the import resolves to, after all. But, what we have is working now, "if it's not broke, don't fix it", and it provides a decent site to add the plugins added by the next entry.

For further enhancements, we've also developed this one:

As is, the wrapping package approach unfortunately does not work well with esbuild treeshaking... but we can utilize esbuild's plugin architecture and the patterns in the backing tslib script to do it ourselves without too much issue. This requires two plugins and an extra build pass for the first:

  1. First phase: Sets write: false, scans for all tslib helper function names used by the codebase, then determines which ones can be safely tree-shaken. An 'ignore' pattern may also be set - we use this to avoid picking up names from a file defining embedded worker code.
  2. Second phase: now that we know which tslib helpers can be eliminated, the second plugin looks for tslib.js and erases the relevant method definitions and the exporter statements.