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
feat: Support .cts
as configuration file
#15283
Changes from 8 commits
350ba10
40b5f2b
25d0fac
0aae8bd
8459ad7
e395515
8a71976
e90dd47
f768926
b76ec12
b34add1
72bbe28
9f79e04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,15 @@ import type { Handler } from "gensync"; | |
import path from "path"; | ||
import { pathToFileURL } from "url"; | ||
import { createRequire } from "module"; | ||
import fs from "fs"; | ||
import semver from "semver"; | ||
|
||
import { endHiddenCallStack } from "../../errors/rewrite-stack-trace"; | ||
import ConfigError from "../../errors/config-error"; | ||
|
||
import { transformSync } from "../../transform"; | ||
import type { InputOptions } from ".."; | ||
|
||
const require = createRequire(import.meta.url); | ||
|
||
let import_: ((specifier: string | URL) => any) | undefined; | ||
|
@@ -23,38 +27,82 @@ export const supportsESM = semver.satisfies( | |
"^12.17 || >=13.2", | ||
); | ||
|
||
export default function* loadCjsOrMjsDefault( | ||
export default function* loadCodeDefault( | ||
filepath: string, | ||
asyncError: string, | ||
// TODO(Babel 8): Remove this | ||
fallbackToTranspiledModule: boolean = false, | ||
): Handler<unknown> { | ||
switch (guessJSModuleType(filepath)) { | ||
case "cjs": | ||
switch (path.extname(filepath)) { | ||
case ".cjs": | ||
return loadCjsDefault(filepath, fallbackToTranspiledModule); | ||
case "unknown": | ||
case ".mjs": | ||
break; | ||
case ".cts": | ||
return loadCtsDefault(filepath); | ||
default: | ||
try { | ||
return loadCjsDefault(filepath, fallbackToTranspiledModule); | ||
} catch (e) { | ||
if (e.code !== "ERR_REQUIRE_ESM") throw e; | ||
} | ||
// fall through | ||
case "mjs": | ||
if (yield* isAsync()) { | ||
return yield* waitFor(loadMjsDefault(filepath)); | ||
} | ||
throw new ConfigError(asyncError, filepath); | ||
} | ||
if (yield* isAsync()) { | ||
return yield* waitFor(loadMjsDefault(filepath)); | ||
} | ||
throw new ConfigError(asyncError, filepath); | ||
} | ||
|
||
function guessJSModuleType(filename: string): "cjs" | "mjs" | "unknown" { | ||
switch (path.extname(filename)) { | ||
case ".cjs": | ||
return "cjs"; | ||
case ".mjs": | ||
return "mjs"; | ||
default: | ||
return "unknown"; | ||
function loadCtsDefault(filepath: string) { | ||
const ext = ".cts"; | ||
const hasTsSupport = !!( | ||
require.extensions[".ts"] || | ||
require.extensions[".cts"] || | ||
require.extensions[".mts"] | ||
Comment on lines
+58
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the plan to eventually support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I want to support this, but it may be difficult, and the current nodejs support for this is not very complete. |
||
); | ||
if (!hasTsSupport) { | ||
const code = fs.readFileSync(filepath, "utf8"); | ||
const opts: InputOptions = { | ||
babelrc: false, | ||
configFile: false, | ||
filename: path.basename(filepath), | ||
sourceType: "script", | ||
sourceMaps: "inline", | ||
presets: [ | ||
[ | ||
"@babel/preset-typescript", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we first try |
||
process.env.BABEL_8_BREAKING | ||
liuxingbaoyu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
? { | ||
disallowAmbiguousJSXLike: true, | ||
allExtensions: true, | ||
onlyRemoveTypeImports: true, | ||
optimizeConstEnums: true, | ||
} | ||
: { | ||
allowDeclareFields: true, | ||
disallowAmbiguousJSXLike: true, | ||
allExtensions: true, | ||
onlyRemoveTypeImports: true, | ||
optimizeConstEnums: true, | ||
}, | ||
], | ||
], | ||
}; | ||
const result = transformSync(code, opts); | ||
liuxingbaoyu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
require.extensions[ext] = function (m, filename) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the require hook be installed before |
||
if (filename === filepath) { | ||
// @ts-expect-error Undocumented API | ||
return m._compile(result.code, filename); | ||
} | ||
return require.extensions[".js"](m, filename); | ||
}; | ||
} | ||
try { | ||
return endHiddenCallStack(require)(filepath); | ||
} finally { | ||
if (!hasTsSupport) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The user config could register a new handler for TS extensions. We should also check if |
||
delete require.extensions[ext]; | ||
} | ||
} | ||
} | ||
|
||
|
@@ -69,8 +117,7 @@ function loadCjsDefault(filepath: string, fallbackToTranspiledModule: boolean) { | |
async function loadMjsDefault(filepath: string) { | ||
if (!import_) { | ||
throw new ConfigError( | ||
"Internal error: Native ECMAScript modules aren't supported" + | ||
" by this platform.\n", | ||
"Internal error: Native ECMAScript modules aren't supported by this platform.\n", | ||
filepath, | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
import { loadPartialConfigSync } from "../lib/index.js"; | ||
import path from "path"; | ||
import { fileURLToPath } from "url"; | ||
import { createRequire } from "module"; | ||
import semver from "semver"; | ||
|
||
const __dirname = path.dirname(fileURLToPath(import.meta.url)); | ||
const require = createRequire(import.meta.url); | ||
|
||
// We skip older versions of node testing for two reasons. | ||
// 1. ts-node does not support the old version of node. | ||
// 2. In the old version of node, jest has been registered in `require.extensions`, which will cause babel to disable the transforming as expected. | ||
|
||
describe("@babel/core config with ts [dummy]", () => { | ||
it("dummy", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this needed because Jest complains if all the tests in the file are skipped? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Can you make it only run on |
||
expect(1).toBe(1); | ||
}); | ||
}); | ||
|
||
semver.gte(process.version, "12.0.0") | ||
? describe | ||
: describe.skip("@babel/core config with ts", () => { | ||
it("should work with simple .cts", () => { | ||
const config = loadPartialConfigSync({ | ||
configFile: path.join( | ||
__dirname, | ||
"fixtures/config-ts/simple-cts/babel.config.cts", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious, does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure, I've encountered a lot of |
||
), | ||
}); | ||
|
||
expect(config.options.targets).toMatchInlineSnapshot(` | ||
Object { | ||
"node": "12.0.0", | ||
} | ||
`); | ||
}); | ||
|
||
it("should throw with invalid .ts register", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a new test case for invalid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is actually a special situation here. I found that |
||
require.extensions[".ts"] = () => { | ||
throw new Error("Not support .ts."); | ||
}; | ||
expect(() => { | ||
loadPartialConfigSync({ | ||
configFile: path.join( | ||
__dirname, | ||
"fixtures/config-ts/invalid-cts-register/babel.config.cts", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the fixture here be |
||
), | ||
}); | ||
}).toThrow(/Unexpected identifier.*/); | ||
delete require.extensions[".ts"]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
}); | ||
|
||
it("should work with ts-node", async () => { | ||
const service = eval("import('ts-node')").register({ | ||
liuxingbaoyu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
experimentalResolver: true, | ||
compilerOptions: { | ||
module: "CommonJS", | ||
}, | ||
}); | ||
service.enabled(true); | ||
|
||
require(path.join( | ||
__dirname, | ||
"fixtures/config-ts/simple-cts-with-ts-node/babel.config.cts", | ||
)); | ||
|
||
const config = loadPartialConfigSync({ | ||
configFile: path.join( | ||
__dirname, | ||
"fixtures/config-ts/simple-cts-with-ts-node/babel.config.cts", | ||
), | ||
}); | ||
|
||
service.enabled(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. The test teardown should be placed in a try-finally block or wrapped in the |
||
|
||
expect(config.options.targets).toMatchInlineSnapshot(` | ||
Object { | ||
"node": "12.0.0", | ||
} | ||
`); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
type config = any; | ||
module.exports = { targets: "node 12.0.0" } as config; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
type config = any; | ||
module.exports = { targets: "node 12.0.0" } as config; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
type config = any; | ||
module.exports = { targets: "node 12.0.0" } as config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exactly how we should do it, but it's also a potential breaking change 🙁
peerDependenciesMeta
is only supported in npm 7+, and Node.js 14 comes with npm 6 preinstalled. There are also other places where we would needpeerDependenciesMeta
, but they are waiting for Babel 8.Maybe we could
try/catch
requiring the preset without listing it as a peer dependency, even if this means that for now it won't work with PnP? Or we could only support loading throughts-node
until Babel 8.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a little hesitant about this, because even if the npm used by the user does not support
peerDependenciesMeta
, it should only install an additional plugin, and there will be no other side effects except for a very small hard disk occupation. 😕There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, in old npm versions that don't support
peerDependenciesMeta
@babel/preset-typescript
will not be installed, and instead it will be handled like a required peer dependency.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get it, this causes a warning to show up in some versions of npm, which is really less than ideal, thanks!