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

Emit for dynamic import (import()) when target >= ES2020 and module == None #48702

Closed
rbuckton opened this issue Apr 14, 2022 · 4 comments · Fixed by #50942
Closed

Emit for dynamic import (import()) when target >= ES2020 and module == None #48702

rbuckton opened this issue Apr 14, 2022 · 4 comments · Fixed by #50942
Assignees
Labels
Committed The team has roadmapped this issue Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone Suggestion An idea for TypeScript

Comments

@rbuckton
Copy link
Member

rbuckton commented Apr 14, 2022

Bug Report

We unconditionally transform dynamic import (import()) regardless of the current script target when the module kind is None. This is due to us using the CommonJS module emit as a fallback for None, and is causing an issue for VSCode while trying to implement support for TS Server plugins on the web via WebWorker.

I propose that we change the emit behavior for import() when the module target is None to preserve import(). Note that this would constitute a breaking change, but I imagine the number of affected projects (those currently using both --module None and import()) would be vanishingly small.

🔎 Search Terms

dynamic import module none node transform emit outFile

⏯ Playground Link

Playground link with relevant code

💻 Code

// @target: es2020
// @module: none
// @outFile: foo.js
const foo = import("foo");

🙁 Actual behavior

Transforms import() into a call to require()

🙂 Expected behavior

Does not transform import().

@rbuckton
Copy link
Member Author

/cc: @DanielRosenwasser, @RyanCavanaugh

@rbuckton rbuckton changed the title Incorrect emit for dynamic import (import()) when target >= ES2020 Emit for dynamic import (import()) when target >= ES2020 and module == None Apr 14, 2022
@craigphicks
Copy link

craigphicks commented Apr 19, 2022

@rbuckton

    export enum ModuleKind {
        None = 0,
        CommonJS = 1,
        AMD = 2,
        UMD = 3,
        System = 4,
        ES2015 = 5,
        ES2020 = 6,
        ESNext = 99
    }

According to https://www.typescriptlang.org/tsconfig#module, the default value is
CommonJS if target is ES3 or ES5, otherwise it is ES6/ES2015.

When you say "... This is due to us using the CommonJS module emit as a fallback for None", what does that mean exactly?
Do you actually set the value to "None" in the tsconfig.json text file?
Or set an internal tsconfig memory object member "module" to ts.ModuleKind.None inside a custom API?

In case of simple running tsc, an missing "module" property in tsconfig.compilerOptions should result in the correct default value internally (not always "CommonJS"). I can see how that might not happen if you are using a custom API. If the correct default is not happening when it should I guess it would be a bug. But the API allows overriding that default value intentionally, or even not intentionally.

@haydentech
Copy link

haydentech commented Jun 21, 2022

Will the fix for this issue include the unconditional suppression of "export {};" from the end of the emitted JavaScript when module is set to None?

@RyanCavanaugh
Copy link
Member

@haydentech that isn't related to this issue

@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Sep 25, 2022
@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone Suggestion An idea for TypeScript
Projects
None yet
5 participants