Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tslib should follow standards for ESM/CJS detection #173

Open
johncrim opened this issue Feb 25, 2022 · 9 comments
Open

tslib should follow standards for ESM/CJS detection #173

johncrim opened this issue Feb 25, 2022 · 9 comments

Comments

@johncrim
Copy link

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
Copy link
Author

johncrim commented Feb 26, 2022

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
Copy link
Author

@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
Copy link

ahnpnl commented Feb 27, 2022

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)

@wmertens
Copy link

wmertens commented Apr 8, 2022

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
Copy link

wmertens commented Apr 8, 2022

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
Copy link

wmertens commented Apr 9, 2022

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

@johncrim
Copy link
Author

johncrim commented Apr 10, 2022

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
Copy link

@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
Copy link

jahorton commented Jun 9, 2023

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 evanw/esbuild#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants