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

esm: Modify ESM Experimental Loader Hooks #34753

Open
julian-londono opened this issue Aug 12, 2020 · 4 comments
Open

esm: Modify ESM Experimental Loader Hooks #34753

julian-londono opened this issue Aug 12, 2020 · 4 comments
Labels
discuss Issues opened for discussions and feedbacks. esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@julian-londono
Copy link

This issue is regarding #34144 . The original proposal on that PR was to modify the getSource loader hook to be able to optionally return a format in its return object that overrides the format returned by getFormat.

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

@DerekNonGeneric DerekNonGeneric added the esm Issues and PRs related to the ECMAScript Modules implementation. label Aug 12, 2020
@jkrems
Copy link
Contributor

jkrems commented Aug 12, 2020

cc @nodejs/modules-active-members with my special ping powers.

@jkrems jkrems added the discuss Issues opened for discussions and feedbacks. label Aug 12, 2020
@MylesBorins MylesBorins added the modules-agenda Issues and PRs to discuss during the meetings of the Modules team. label Aug 13, 2020
@MylesBorins MylesBorins removed the modules-agenda Issues and PRs to discuss during the meetings of the Modules team. label Oct 7, 2020
@MylesBorins
Copy link
Contributor

Removing agenda label. Please re-add if there is more for us to discuss in the meeting

@lostpebble
Copy link

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 format: "commonjs" without knowing that it doesn't actually work- just assumed it might. So it does feel like a natural API to me.

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 resolve hook, and rewrite them. But didn't seem to get it right. Would be nice if this was made a little easier too- perhaps with a recognizedModuleExtensions: [] option or something. But that's probably not completely relevant to this issue.

@jkrems
Copy link
Contributor

jkrems commented Nov 30, 2020

I actually tried returning format: "commonjs" without knowing that it doesn't actually work- just assumed it might.

I think at the least we should issue a warning when a source is provided with format=commonjs, with or without this PR. Because of the async nature of the ESM system, we'd likely be unable to use the result in a "real" require (from CJS) and we wouldn't want to risk a race condition between require and import of the same file.

But you bring up a good general point - we should document how to write a transform that can handle both CJS and ESM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

No branches or pull requests

5 participants