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

Support --out-extension when without bundle #2600

Closed
abetoots opened this issue Oct 11, 2022 · 5 comments
Closed

Support --out-extension when without bundle #2600

abetoots opened this issue Oct 11, 2022 · 5 comments

Comments

@abetoots
Copy link

abetoots commented Oct 11, 2022

Using the Build API caused a lot of problems for us when using ESM in node.

We're running npx esbuild `find server/src \\( -name '*.ts' -o -name '*.mts' \\)` --platform=node --format=esm --outdir=server/dist
as we've learned in #263 .

The problem is the output files are in .js and the emitted code contains imports to .mjs files.

Here is our tsconfig

{
  "compilerOptions": {
    "target": "ESNext",
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "isolatedModules": true,
    "baseUrl": ".",
    "skipLibCheck": true,
    "outDir": "dist",
    "paths": {
      "@globalShared/*": ["../shared/*"]
    }
  },
  "exclude": ["dist", "node_modules"],
  "include": ["./src"]
}

This would've been solved if --out-extension was supported by the transform API.

@thgh
Copy link

thgh commented Oct 17, 2022

Hi, not sure if my issue is the same, but it seems related. I have typescript files (ts/tsx) and would like to publish them as cjs&esm and keep it treeshakeable (so no bundle).

esbuild ./src/* --outdir=esm --out-extension:.js=.mjs

It seems that out-extension is outputting the correct extensions, but it breaks the imports as these are explicit. From esbuild docs:

Node's resolution algorithm doesn't treat these as implicit file extensions, so esbuild doesn't either.

I think it would make more sense that if out-extension ends with .mjs, it should rewrite the import/export statements to valid imports (ending in .mjs).

@evanw
Copy link
Owner

evanw commented Oct 17, 2022

I think it would make more sense that if out-extension ends with .mjs, it should rewrite the import/export statements to valid imports (ending in .mjs).

I'm not convinced doing something like this is a good idea. By design, the transform API doesn't access the file system and doesn't run import resolution, so it doesn't know where a given import will resolve. Blindly changing imports like foo/bar into foo/bar.mjs doesn't mean things will "just work" because node's path resolution algorithm is complex. An Importing foo/bar could resolve into foo/bar.mjs but also foo/bar.js, foo/bar/index.js, or even foo/something/else.js if foo has a package.json file with an exports field.

Edit: By far the simplest solution is to just write .mjs in your imports if you want .mjs in your imports. A more complex solution would be to use esbuild's bundle API with a plugin that adds .mjs to the relevant imports. You won't want to add .mjs to all of them, so this gives you the opportunity to write the custom code to specify exactly which imports you want to add .mjs to.

@thgh
Copy link

thgh commented Oct 17, 2022

If I write .mjs in my imports, can I use esbuild to get a cjs build from it?

Note to self, conditions for adding .mjs:

  • --out-extension:.js=.mjs
  • --target=node18 or higher or browser (or anything?)
  • import starts with ./ or ../

Not sure if package imports have impact.

Today I'm using the script below to add .mjs to relative paths. Will consider to wrap it into a plugin.

const { readFile, writeFile } = require('fs')
for (const filename of process.argv.slice(2)) {
  readFile(filename, 'utf-8', function (err, contents) {
    if (err) return console.log(err)
    contents = contents
      .replaceAll(/(import[{}\sa-z,A-Z0-9]+from "\.\/[^"]+)"/gm, '$1.mjs"')
      .replaceAll(/(export[{}\sa-z,A-Z0-9*]+from "\.\/[^"]+)"/gm, '$1.mjs"')
      .replaceAll('format from "date-fns/format', '{ format } from "date-fns')
    writeFile(filename, contents, 'utf-8', err => err && console.log(err))
  })
}

@evanw
Copy link
Owner

evanw commented Nov 27, 2022

The way to do this is to use the build API with a plugin that customizes the import paths to match your requirements. Documentation for the relevant plugin API is here: https://esbuild.github.io/plugins/#on-resolve.

I'm closing this issue because I don't believe this behavior should be built in to esbuild, as it's error-prone and likely needs to be customized for each use case.

@evanw evanw closed this as not planned Won't fix, can't repro, duplicate, stale Nov 27, 2022
@Tobbe
Copy link

Tobbe commented Apr 7, 2024

I'm running into this same problem. I tried implementing the renaming from .js to .cjs and .mjs using a plugin, but because I'm not bundling it seems my onResolve callback is never invoked with any kind: 'import-statement' paths. All I'm seeing is kind: 'entry-point' with paths like ./src/some/folder/fileName.ts. So the plugin never gets a chance to modify the import paths.

Am I misunderstanding something here?

Here's the plugin I tried with

const jsToMjs: Plugin = {
  name: 'js-to-mjs',
  setup(build) {
    // Find all relative imports (starting with ".") that ends with ".js" and
    // replace the file extension with ".mjs"
    build.onResolve({ filter: /^..+\.js$/ }, (args) => {
      return {
        path: path.join(args.resolveDir, args.path.slice(0, -3) + '.mjs'),
      }
    })
  },
}

But, as I said, the callback is never called, because all the files are just .ts and .tsx entry-point files

(To debug I changed to regex to /./ to match all files)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants