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

Unable to preserve dynamic imports with transform or transformSync API #1029

Closed
marvinhagemeister opened this issue Mar 21, 2021 · 2 comments
Labels

Comments

@marvinhagemeister
Copy link

marvinhagemeister commented Mar 21, 2021

I'm currently using to esbuild to hook into node's module loader via esbuild-register to compile files on demand when they are loaded in node. Internally, it relies on transformSync to transpile files sent by node's internal module loader.

The app is mainly written in commonjs, but loads some esm files via dynamic import statements. To allow node to load esm files from commonjs one must use the dynamic import statement. In esbuild this is supported via the --platform=node flag in cli or via a build property of the same name for build and buildSync APIs.

But no such option exists for transform or transformSync. Those APIs only support setting format=cjs which transpiles every import, including dynamic import statements. That's great for browsers but unwanted in node environments. So there needs to be some way to preserve dynamic import statements via transform and transformSync.

Input code:

// input
export const foo = () => import('./bar.mjs')

Expected output code:

const foo = () => import("./bar.mjs")));

Actual output code:

// ...snip bunch of esm interop code
const foo = () => Promise.resolve().then(() => __toModule(require("./bar.mjs")));
@evanw
Copy link
Owner

evanw commented Mar 21, 2021

I think at some point when I was writing this I didn't realize that import() was a regular function in script mode. Also the TypeScript compiler converts import() to require() when the output format is CommonJS, so I have done that as well. I agree that this would be something good to change. However, this seems like a breaking change to me, so it should wait until the next breaking change release.

@marvinhagemeister
Copy link
Author

Agree, I think that's an error in TypeScript too. There is a relevant issue on their tracker: microsoft/TypeScript#43329

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

No branches or pull requests

2 participants