size-limit icon indicating copy to clipboard operation
size-limit copied to clipboard

support NodeJS ESM Module only project

Open unional opened this issue 3 years ago • 14 comments

Currently, ESM Module only project doesn't work with size-limit, both esbuild and webpack plugin fails to recognize the project:

// package.json
{
  "type": "module",
  "exports": {
    ".": {
      "import": "./lib/index.js"
    }
  }
}

They both try to look for <proj>/index.js because main was not specified.

unional avatar May 27 '22 16:05 unional

As quick fix you can specify path to JS file with path option in Size Limit config.

I agree that it will be nice to look for default path in exports too. Can I ask to send PR? I am working on another project right now.

ai avatar May 27 '22 16:05 ai

Thanks. I take a quick look into the code, but couldn't identify where does it get the default path. :(

btw, is there a esbuild-why similar to webpack-why? (Or if there is a way to get the output file so we can examine it manually)

I have a project that while small, it reports:

✔ Adding to empty esbuild project

  Size limit: 5 kB
  Size:       313 B with all dependencies, minified and gzipped

I'm not sure it is that small~ :)

When I do "path": "./esm/**/*.js" instead, it gives:

✔ Adding to empty esbuild project

  Size limit: 5 kB
  Size:       986 B with all dependencies, minified and gzipped

unional avatar May 28 '22 05:05 unional

For ESM, you need to specify import with list of exports to test (because of esbuild tree-shaking).

ai avatar May 28 '22 07:05 ai

For ESM, you need to specify import

Turns out I don't have to. The package is that small. :)

On the other hand, I'm getting these warnings from esbuild when testing the commonJS output in "ECMAScript module`:

Adding to empty esbuild project▲ [WARNING] Top-level "this" will be replaced with undefined since this file is an ECMAScript module

    cjs/index.js:2:23:
      2 │ var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
        │                        ~~~~
        ╵                        undefined

  This file is considered to be an ECMAScript module because the enclosing "package.json" file sets
  the type of this file to "module":

    package.json:19:10:
      19 │   "type": "module",
   
[WARNING] The CommonJS "exports" variable is treated as a global variable in an ECMAScript module and may not work as expected

    cjs/index.js:17:0:
      17 │ exports.tersify = void 0;
      

These output files are used by CommonJS, where the package is configured as follows:

  "type": "module",
  "exports": {
    "types": "./esm/index.d.ts",
    "import": "./esm/index.js",
    "require": "./cjs/index.js"
  },
  "main": "cjs/index.js",

Is there a way to tell esbuild to treat the input as CommonJS?

unional avatar Jun 02 '22 07:06 unional

It should detects it automatically by package.type

ai avatar Jun 02 '22 07:06 ai

It should detects it automatically by package.type

It does, which is the problem here.

because under type: module, you can export both ESM and CommonJS (as seen in the example above).

As for size-limit usage, I'm checking both:

"size-limit": [
  { "path": "./cjs/index.js", "limit": "5kB" },
  { "path": "./esm/index.js", "limit": "5kB" }
]

unional avatar Jun 02 '22 09:06 unional

You need to explicitly add .cjs extension for CJS file (or add package.json to ./cjs/).

Otherwise, your npm package will not work in Node.js.

ai avatar Jun 02 '22 09:06 ai

TypeScript (including the latest 4.7) does not support that.

unional avatar Jun 02 '22 09:06 unional

Welcome to the hell of ESM migration :D. I can’t help with TS.

ai avatar Jun 02 '22 09:06 ai

🤣 no problem. ESM migration with TypeScript is hell x 2.

Here are some links related to it if you are interested:

https://github.com/microsoft/TypeScript/issues/49083 https://github.com/microsoft/TypeScript/issues/49271 https://github.com/microsoft/TypeScript/issues/49335

unional avatar Jun 02 '22 18:06 unional

ESM migration with TypeScript and Jest is hell^3.

@unional, if understand correctly, it seems that's necessary to inject .js extensions into .ts/.d.ts but rename target/cjs/*.js to.cjs.

antongolub avatar Jun 06 '22 12:06 antongolub

@unional, if understand correctly, it seems that's necessary to inject .js extensions into .ts/.d.ts but rename target/cjs/*.js to.cjs.

What I understand is to these two things:

  • add .js to .ts import statement
  • create cjs/package.json containing { "type": "commonjs" }

I'm still not sure about changing the .d.ts and cjs/*.js post transpilation. Because that could break sourceMap, inlineSourceMap, and inlineSources.

I think that's also part of the reason TypeScript team don't want to do it.

unional avatar Jun 06 '22 16:06 unional

We can use import() instead of require() easily to support ESM only packages AFAIK, I'd like to try to raise a PR for it.

JounQin avatar Jul 31 '22 00:07 JounQin

@unional

I'm still not sure about changing the .d.ts and cjs/*.js post transpilation.

See https://github.com/sheremet-va/dual-packaging

You need something like rollup

JounQin avatar Jul 31 '22 00:07 JounQin