-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
esm: Modify ESM Experimental Loader Hooks #34753
Comments
cc @nodejs/modules-active-members with my special ping powers. |
Removing agenda label. Please re-add if there is more for us to discuss in the meeting |
Just thought I'd drop by to say I've run into a bit of a wall today, which I think these coming changes would have been very helpful for: // Typescript files
const extensionsRegex = /\.ts$|\.tsx$/;
export function transformSource(source, context, defaultTransformSource) {
const { url } = context;
if (extensionsRegex.test(url)) {
return {
source: babel.transform(source, {
rootMode: "upward",
cwd: process.cwd(),
filename: url,
sourceType: "unambiguous",
ast: false
}).code,
format: "commonjs"
};
}
// Let Node.js handle all other sources.
return defaultTransformSource(source, context, defaultTransformSource);
} I actually tried returning I also ran into some issues with Node not being able to recognize relative TypeScript imports without an extension, for example: import "./ServerConfig"; I tried to intercept the imports with the |
I think at the least we should issue a warning when a source is provided with But you bring up a good general point - we should document how to write a transform that can handle both CJS and ESM. |
This issue is regarding #34144 . The original proposal on that PR was to modify the
getSource
loader hook to be able to optionally return aformat
in its return object that overrides the format returned bygetFormat
.This method was eventually discussed at a team meeting: nodejs/modules#536 and a new proposal was eventually brought up: remove the
getFormat
hook entirely.I plan to explore this option and submit a PR in the coming 2 weeks.
cc: @jkrems @nodejs/modules-active-members
The text was updated successfully, but these errors were encountered: