feat(typescript): warn when compilerOptions.module is not esnext #788
Conversation
|
@benmccann you may also be interested in this change |
packages/typescript/src/preflight.ts
Outdated
| // eslint-disable-next-line import/prefer-default-export | ||
| export const preflight = (config: TypeScriptConfig, context: PluginContext) => { | ||
| let undef; | ||
| if (![ModuleKind.ESNext, ModuleKind.ES2020, undef].includes(config.options.module)) { |
There was a problem hiding this comment.
it's not in the list of supported module types. I went with the TS definition for ModuleKind
There was a problem hiding this comment.
I think this is too restrictive. Besides those, ES6 and ES2015 are also perfectly fine for use with Rollup as they also imply the use of import/export but just do not support dynamic import statements.
There was a problem hiding this comment.
In that light, maybe the wording of the message could be changed: ...Unfortunately your configuration specifies a
"module" that does not support that. ...
There was a problem hiding this comment.
(for reference for onlookers https://github.com/microsoft/TypeScript/blob/79c72751df2aaed2b2ba8caa4170895c343a3d5b/lib/protocol.d.ts#L2598-L2607)
as long as we're sure that ES6 and ES2015 are valid, I'm good to add them.
There was a problem hiding this comment.
Also in the TypeScript documentation, they usually write ESNext, but I think this is more like a style point.
packages/typescript/src/preflight.ts
Outdated
| // eslint-disable-next-line import/prefer-default-export | ||
| export const preflight = (config: TypeScriptConfig, context: PluginContext) => { | ||
| let undef; | ||
| if (![ModuleKind.ESNext, ModuleKind.ES2020, undef].includes(config.options.module)) { |
There was a problem hiding this comment.
Is there a benefit using a variable over undefined?
There was a problem hiding this comment.
elegantly gets around linter issue
| in the target tsconfig.json file or plugin options.`.replace(/\n/g, ''); | ||
|
|
||
| let undef; | ||
| const validModules = [ModuleKind.ES2015, ModuleKind.ES2020, ModuleKind.ESNext, undef]; |
There was a problem hiding this comment.
@lukastaegert @NotWoods Note here that ES6 isn't available in this context. There seems to be some weird fragmentation in the TypeScript source. Here's the code from typescript.js that define the available values:
(function (ModuleKind) {
ModuleKind[ModuleKind["None"] = 0] = "None";
ModuleKind[ModuleKind["CommonJS"] = 1] = "CommonJS";
ModuleKind[ModuleKind["AMD"] = 2] = "AMD";
ModuleKind[ModuleKind["UMD"] = 3] = "UMD";
ModuleKind[ModuleKind["System"] = 4] = "System";
// NOTE: ES module kinds should be contiguous to more easily check whether a module kind is *any* ES module kind.
// Non-ES module kinds should not come between ES2015 (the earliest ES module kind) and ESNext (the last ES
// module kind).
ModuleKind[ModuleKind["ES2015"] = 5] = "ES2015";
ModuleKind[ModuleKind["ES2020"] = 6] = "ES2020";
ModuleKind[ModuleKind["ESNext"] = 99] = "ESNext";
})(ModuleKind = ts.ModuleKind || (ts.ModuleKind = {}));There was a problem hiding this comment.
ES6 likely gets transformed into ES2015 when the strings are converted into the equivalent enum values.
Rollup Plugin Name:
typescriptThis PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers: Fixes #519
Description
This PR adds a warning when the
typescriptplugin is passed configuration with"module"set to a value other than"esnext"or"es2020". Please see the snapshots for the phrasing of the warning message. This was not made an error due to the existence of edge cases for advanced compiling in which someone may actually want to outputcommonjs.