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

esbuild@>=0.14.5 introduces a regression for __toESM(require('esbuild')) #1897

Closed
Kiyozz opened this issue Dec 31, 2021 · 10 comments
Closed

Comments

@Kiyozz
Copy link

Kiyozz commented Dec 31, 2021

Versions  
First repro 0.14.9
Also repro latest, >=0.14.5
Not repro 0.13.x, <0.14.5
OS Windows
Node/npm 17

Repro repository - use-tsup branch

  1. git checkout f336527bb964c0933a25168b821f334bd52c7a55
  2. pnpm i
  3. cd packages/ee - pnpm prepublishOnly
  4. cd ../electron-esbuild - pnpm prepublishOnly

Used command to build ee package : esbuild src/index.ts --outdir=dist --bundle --platform=node --external:esbuild --sourcemap. Note esbuild as external

CURRENT: Uncaught TypeError: Cannot read properties of undefined (reading 'build')
EXPECTED: no error

This is working with esbuild 0.14.3, redo all above with cd packages/ee - pnpm i -D esbuild@0.14.3

The problem is that esbuild changes the source from

import esbuild from 'esbuild'
...
esbuild.build({...})

to

var import_esbuild = __toESM(require('esbuild'))
...
import_esbuild.default.build({...})

But from esbuild@0.14.5, var import_esbuild = __toESM(require('esbuild')).default is undefined but not with esbuild@<0.14.5

Note that, this is working using import { build } from 'esbuild' because bundled source do not use import_esbuild.default.build but import_esbuild.build directly

@Kiyozz Kiyozz changed the title esbuild >=0.14.5 introduces a regression for __toESM(require('esbuild')) esbuild@>=0.14.5 introduces a regression for __toESM(require('esbuild')) Dec 31, 2021
@evanw
Copy link
Owner

evanw commented Dec 31, 2021

This is because there's no export called default in esbuild's source code. Both your code and esbuild's code is in ES module format, so your code would be a run-time import binding failure if everything was run as ES modules. Everything is being run as CommonJS instead so there's no run-time import binding failure, but esbuild still emulates ES module semantics on top of CommonJS so doing this still won't work. You have a few options:

  1. Don't use the default export, since it doesn't exist:

    import * as esbuild from 'esbuild'
  2. Use require() instead of using import, which gives you the underlying import directly:

    import esbuild = require('esbuild')
  3. Tell esbuild to compile the file build.ts in node compatibility mode, since node invents a default export in this case. You can do this by changing the file name to build.mts, which esbuild takes as a signal that it should use node's .mjs semantics.

@Kiyozz
Copy link
Author

Kiyozz commented Dec 31, 2021

Yes, esbuild does not have a default export. But the job of __toESM is to add a .default in case the module does not have one? If so, import_esbuild.default should not be undefined. Am I wrong?

@hyrious
Copy link

hyrious commented Jan 1, 2022

esbuild is aligned to the babel/webpack behavior of es module interop since 0.14.5. That is to say, if a cjs file has __esModule: true, you will get the exports.default value when import default_value. This is for compatibility with other bundlers and keeping the es modules semantic consist regardless of platform (node/browser).

In fact, node's behavior is getting more complex during the days. In node.js 17, importing a cjs module will give you not only the whole module.exports as default export, but also some named export guessing with cjs-module-lexer, while that's a runtime feature, bundler's can not repeat that.

In conclusion, esbuild (and other bundlers) will be consist with their esm interop rule when transforming esm to cjs. If you want to get the node.js behavior, why not just output esm (format: esm) and let node run that?

@evanw
Copy link
Owner

evanw commented Jan 1, 2022

But the job of __toESM is to add a .default in case the module does not have one?

This is incorrect. The job of __toESM is to implement esbuild's ESM interop behavior. That behavior only involves overwriting the default export when the file is in node compatibility mode. Node compatibility mode is only enabled if the file extension is .mjs or .mts, or if the enclosing package.json file contains "type": "module". Otherwise, the behavior of a converted ESM import statement is designed to emulate the behavior of the original ESM code before the conversion.

@Kiyozz
Copy link
Author

Kiyozz commented Jan 1, 2022

Thanks both of you. It must bo clear that typescript's esModuleInterop cannot be used with esbuild. Because typescript will add the .default if the module does not defined a default, but esbuild will not (but code will compile and type check successfully). Is this something "normal" or a warn should be throwned?

@hyrious
Copy link

hyrious commented Jan 1, 2022

TypeScript doesn't add .default if __esModule is true, neither will he add default export if your input is in esm, Playground Link. The result cjs code is not able to run too.

@Kiyozz
Copy link
Author

Kiyozz commented Jan 1, 2022

Okay!! I was missing the mod && mod.__esModule.

Shouldn't this version (0.14.5) have been a breaking change then?

@evanw
Copy link
Owner

evanw commented Jan 6, 2022

Shouldn't this version (0.14.5) have been a breaking change then?

Perhaps. Technically every version is a breaking change for people relying on the old behavior. It's up to the publisher what is considered "bug fix" vs. "breaking change" even though bug fixes can break code that makes use of the bug. It seemed like a bug fix to me (specifically #1719, #1591, and #532).

@Kiyozz
Copy link
Author

Kiyozz commented Jan 6, 2022

Shouldn't this version (0.14.5) have been a breaking change then?

Perhaps. Technically every version is a breaking change for people relying on the old behavior. It's up to the publisher what is considered "bug fix" vs. "breaking change" even though bug fixes can break code that makes use of the bug. It seemed like a bug fix to me (specifically #1719, #1591, and #532).

I would say, only in the case where the old behavior was not the good one. If in the first place, le old behavior was just a mistake in the implementation, then it is a breaking. But if the old behavior should have worked like the new one, then it is a bug.

@aminick
Copy link

aminick commented Jan 23, 2022

Currently experiencing this with "styled-components" too, which as a popular library, I suspect a lot of people have the same issue? upvote if you do, or I made a mistake on my end.

Setup:

"styled-components": "^5.3.3"
"tsup": "^5.11.11",
"vite": "^2.7.2"

Im using tsup to bundle the library, thus im here.

I suspect this is causing the issue?
image

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