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

support NodeJS ESM Module only project #285

Open
unional opened this issue May 27, 2022 · 14 comments
Open

support NodeJS ESM Module only project #285

unional opened this issue May 27, 2022 · 14 comments

Comments

@unional
Copy link
Contributor

unional commented May 27, 2022

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.

@ai
Copy link
Owner

ai commented May 27, 2022

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.

@unional
Copy link
Contributor Author

unional commented May 28, 2022

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

@ai
Copy link
Owner

ai commented May 28, 2022

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

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

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?

@ai
Copy link
Owner

ai commented Jun 2, 2022

It should detects it automatically by package.type

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

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" }
]

@ai
Copy link
Owner

ai commented Jun 2, 2022

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.

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

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

@ai
Copy link
Owner

ai commented Jun 2, 2022

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

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

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

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

microsoft/TypeScript#49083
microsoft/TypeScript#49271
microsoft/TypeScript#49335

@antongolub
Copy link

antongolub commented Jun 6, 2022

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.

@unional
Copy link
Contributor Author

unional commented Jun 6, 2022

@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.

@JounQin
Copy link
Contributor

JounQin commented Jul 31, 2022

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

JounQin commented Jul 31, 2022

@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

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