Fix support for `import/export` in `.cts` files #15478

liuxingbaoyu · 2023-03-09T15:42:31Z

Q	A
Fixed Issues?
Patch: Bug Fix?	?
Major: Breaking Change?	Probably not, but not 100%
Minor: New Feature?
Tests Added + Pass?	√
Documentation PR Link
Any Dependency Changes?	√
License	MIT

liuxingbaoyu · 2023-03-09T15:44:22Z

packages/babel-preset-typescript/src/index.ts

-            plugins: [[transformTypeScript, pluginOptions(false, true)]],
+            parserOpts: {
+              allowImportExportEverywhere: true,
+            },
+            plugins: [
+              [transformModulesCommonJS, { allowTopLevelThis: true }],
+              [transformTypeScript, pluginOptions(false, true)],
+            ],


This is probably not good, since it means incompatibility with allExtensions. I want to know your opinion.

Would updating this preset to only specify the unambiguous source type and keep the modules transform out of the preset work?

babel.config.cts requires it, and almost all cts files require this. Because when using require instead of import, tsc cannot get type information, even for built-in modules like fs.

liuxingbaoyu · 2023-03-09T15:51:01Z

Do we have any existing way to test @babel/preset-typescript in workspace? Now it loads plugins on npm forever.

nicolo-ribaudo · 2023-03-09T16:59:26Z

packages/babel-core/src/config/files/module-types.ts

    const opts: InputOptions = {
      babelrc: false,
      configFile: false,
      sourceType: "script",


I think .cts files are supposed to be parsed as "unambiguous". If I try this CTS file:

export {}; with ({});

TS complains that with cannot be used in strict mode, so it's parsing it as a module.

packages/babel-core/src/config/files/module-types.ts

nicolo-ribaudo · 2023-03-09T17:26:44Z

Do we have any existing way to test @babel/preset-typescript in workspace? Now it loads plugins on npm forever.

I usually copy-paste it outside of the repo (so that it doesn't actually build itself), and update package.json to use it from the local filesystem.

babel.config.js

babel-bot · 2023-03-09T17:32:13Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/54191/

packages/babel-preset-typescript/src/index.ts

nicolo-ribaudo · 2023-03-09T17:53:01Z

packages/babel-core/src/config/files/module-types.ts

+    const packageJson = require("@babel/preset-typescript/package.json");
+
+    if (
+      semver.lte(


I would describe previous versions 7.21.0 as "they supports cts files, but it has a bug that makes it not work with imports/exports". This would also avoid the breakage of throwing for whoever is already using babel.config.cts files with an older version of the preset (which works for all the simple self-contained configs)

nicolo-ribaudo · 2023-03-09T17:54:00Z

packages/babel-helper-module-imports/src/is-module.ts

-  const { sourceType } = path.node;
-  if (sourceType !== "module" && sourceType !== "script") {
-    throw path.buildCodeFrameError(
-      `Unknown sourceType "${sourceType}", cannot transform.`,
-    );
-  }
-


Why do we need this change?

Now sourceType must be one of the two, unless one uses babal-7-beta and writes a new bug to trigger this.

liuxingbaoyu · 2023-03-12T15:52:11Z

CI failures are related.
Do we need to fix @babel/preset-typescript for esm versions now? Or skip this test.

nicolo-ribaudo · 2023-03-13T11:59:58Z

Do we need to fix @babel/preset-typescript for esm versions now? Or skip this test.

.cts configs are already broken in Babel 8, because we already cannot synchronously load the preset, right? We can skip the test for now.

I'm still not convinced that enabling the CJS transform in the preset is a good idea, but I'm playing a bit with tsc to either:

convince myself that it's ok to do it, or
give a proper reason to why I think it's not a good idea.

liuxingbaoyu · 2023-03-14T11:56:51Z

microsoft/TypeScript#51479 (comment)

No transformations between module systems
True to its name, verbatimModuleSyntax has another consequence: ESM syntax cannot be used in files that will emit CommonJS syntax. For example:

import { writeFile } from "fs";
This import is legal under --module esnext, but an error in --module commonjs. (In node16 and nodenext, it depends on the file extension and/or the package.json "type" field.) If the file is determined to be a CommonJS module at emit by any of these settings, it must be written as

import fs = require("fs");
instead. Many users have the impression that this syntax is legacy or deprecated, but that’s not the case. It accurately reflects that the output will use a require statement, instead of obscuring the output behind layers of transformations and interop helpers. I think using this syntax is particularly valuable in .cts files under --module nodenext, because in Node’s module system, imports and requires have markedly different semantics, and actually writing out require helps you understand when and why you can’t require an ES module—it’s easier to lose track of this when your require is disguised as an ESM import in the source file.

That could be a reason, but this option doesn't seem to be used by many people.

liuxingbaoyu · 2023-03-14T19:56:13Z

CI is a regression of node v19.8.0.

nicolo-ribaudo · 2023-03-15T10:56:20Z

The result of my analysis with TSC is that there is absolutely no difference between .cts and .mts files:

even if they don't contain imports or exports, they are always parsed as modules (the output generated by tsc always sets __esModule)
the extension of the file has no effect on how modules are compiled:
- both in .cts and .mts files, if you don't specify the "module" option (or if you specify "module": "CommonJS", which is the default value) you get a CommonJS output
- both in .cts and .mts files, if you don't specify the "module": "ES2022" option you get an ESM output
regardless of how the module is transformed, .cts files are always converted to .cjs files (even if they might contain ESM statements) and .mts files are always converted to .mjs files (even if they might contain CJS statements)

TypeScript's "module": "ES****" option has always been equivalent to "don't compile modules with Babel" and its "module": "CommonJS" option has always been equivalent to "add @babel/plugin-transform-modules-commonjs, or make sure that @babel/preset-env is compiling to CommonJS". As such, I would still prefer to not include the CommonJS plugin in our TypeScript preset.

However, since users cannot change the config we use to load babel.config.cts files, I think we should include @babel/plugin-transform-modules-commonjs in @babel/core (maybe only if it's installed, similarly to how we are checking if the TS preset is installed?):

if users do not need to use imports/exports in their TS Babel config, they will not notice any difference
if they want to use imports/exports, they can do so by installing the plugin to compile them to CommonJS (or, we might just add it as a dependency of @babel/core and make it work by default maybe?)

liuxingbaoyu · 2023-03-15T11:08:25Z

3. regardless of how the module is transformed, .cts files are always converted to .cjs files (even if they might contain ESM statements)

This is really weird, I can't figure out what to do with .cjs with import.

if users do not need to use imports/exports in their TS Babel config, they will not notice any difference

This may be relatively rare, especially in babel.config.cts, only the configuration file is complex enough to use ts to have a significant benefit.

if they want to use imports/exports, they can do so by installing the plugin to compile them to CommonJS (or, we might just add it as a dependency of @babel/core and make it work by default maybe?)

It also looks odd to include @babel/plugin-transform-modules-commonjs in @babel/core. 😕
I'd rather release a new package that includes @babel/plugin-transform-modules-commonjs and ts.

Anyway thanks a lot for these results, I was also very curious but never did.🤦‍♂️

nicolo-ribaudo · 2023-03-22T23:51:34Z

@Andarist showed me that when using the intended TS "module" option for cts and mts files ("module": "Node16"), TS converts modules to CJS in .cts files and not in .mts files: nicolo-ribaudo/ts-experiment-cts-mts-behavior#1

I'm not opposed anymore to including the CommonJS transform in the preset, when compiling .cts files.

liuxingbaoyu · 2023-03-23T18:31:18Z

packages/babel-core/src/config/files/module-types.ts

+              semver.lte(
+                // eslint-disable-next-line import/no-extraneous-dependencies
+                packageJson.version,
+                "7.21.0",


We should change this before merging.

Before releasing, right? Could you add a notice in new-version-check in the makefile?

nicolo-ribaudo · 2023-03-28T08:41:03Z

packages/babel-core/src/config/files/module-types.ts

+              semver.lte(
+                // eslint-disable-next-line import/no-extraneous-dependencies
+                packageJson.version,
+                "7.21.0",


Before releasing, right? Could you add a notice in new-version-check in the makefile?

Copied from babel#15478 Co-authored-by: liuxingbaoyu <[email protected]>

JLHwung

LGTM pending the makefile new-version notes.

liuxingbaoyu · 2023-03-30T16:12:05Z

@nicolo-ribaudo Thanks!

liuxingbaoyu added i: discussion area: typescript labels Mar 9, 2023

liuxingbaoyu commented Mar 9, 2023

View reviewed changes

liuxingbaoyu changed the title ~~fix: import/export for .cts support~~ fix: Support import/export for .cts Mar 9, 2023

nicolo-ribaudo reviewed Mar 9, 2023

View reviewed changes

packages/babel-core/src/config/files/module-types.ts Show resolved Hide resolved

liuxingbaoyu commented Mar 9, 2023

View reviewed changes

babel.config.js Outdated Show resolved Hide resolved

liuxingbaoyu marked this pull request as ready for review March 9, 2023 17:33

nicolo-ribaudo reviewed Mar 9, 2023

View reviewed changes

packages/babel-preset-typescript/src/index.ts Outdated Show resolved Hide resolved

liuxingbaoyu force-pushed the fix-cts branch 2 times, most recently from 0776deb to fbcc25c Compare March 9, 2023 17:48

nicolo-ribaudo reviewed Mar 9, 2023

View reviewed changes

nicolo-ribaudo mentioned this pull request Mar 15, 2023

[ts] Support import ... = and export = in scripts #15497

Merged

liuxingbaoyu mentioned this pull request Mar 23, 2023

chore: Use babel.config.cts #15511

Closed

liuxingbaoyu force-pushed the fix-cts branch from f28c963 to fa135ed Compare March 23, 2023 18:27

liuxingbaoyu added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Mar 23, 2023

liuxingbaoyu commented Mar 23, 2023

View reviewed changes

nicolo-ribaudo approved these changes Mar 28, 2023

View reviewed changes

nicolo-ribaudo added a commit to nicolo-ribaudo/babel that referenced this pull request Mar 29, 2023

Skip cts config tests since they are not working yet

0eaf3b7

Copied from babel#15478 Co-authored-by: liuxingbaoyu <[email protected]>

JLHwung approved these changes Mar 29, 2023

View reviewed changes

nicolo-ribaudo force-pushed the fix-cts branch from fa135ed to ced460c Compare March 30, 2023 13:51

liuxingbaoyu and others added 6 commits March 30, 2023 16:03

fix

8ceca8e

fix

682baf2

review and fix

bb12ab0

review

2dd15b2

Fix after rebase

092f0ad

new-version-checklist

c6c78ae

nicolo-ribaudo force-pushed the fix-cts branch from ced460c to c6c78ae Compare March 30, 2023 14:04

nicolo-ribaudo removed the i: discussion label Mar 30, 2023

nicolo-ribaudo changed the title ~~fix: Support import/export for .cts~~ Fix support for import/export in .cts files Mar 30, 2023

nicolo-ribaudo merged commit fad4e77 into babel:main Mar 30, 2023

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 30, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2023

Uh oh!

Fix support for import/export in .cts files #15478

Fix support for import/export in .cts files #15478

Uh oh!

Conversation

liuxingbaoyu commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuxingbaoyu commented Mar 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicolo-ribaudo commented Mar 9, 2023

Uh oh!

Uh oh!

babel-bot commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuxingbaoyu Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuxingbaoyu commented Mar 12, 2023

Uh oh!

nicolo-ribaudo commented Mar 13, 2023

Uh oh!

liuxingbaoyu commented Mar 14, 2023

Uh oh!

liuxingbaoyu commented Mar 14, 2023

Uh oh!

nicolo-ribaudo commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liuxingbaoyu commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo commented Mar 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLHwung left a comment

Choose a reason for hiding this comment

Uh oh!

liuxingbaoyu commented Mar 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix support for `import/export` in `.cts` files #15478

Fix support for `import/export` in `.cts` files #15478

liuxingbaoyu commented Mar 9, 2023 •

edited

Loading

babel-bot commented Mar 9, 2023 •

edited

Loading

liuxingbaoyu Mar 9, 2023 •

edited

Loading

nicolo-ribaudo commented Mar 15, 2023 •

edited

Loading

liuxingbaoyu commented Mar 15, 2023 •

edited

Loading