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

The right way to modify bundle output #2791

Closed
awxalbert opened this issue Jan 4, 2023 · 3 comments
Closed

The right way to modify bundle output #2791

awxalbert opened this issue Jan 4, 2023 · 3 comments

Comments

@awxalbert
Copy link

It's a follow-up of this issue, since I haven't received a reply for weeks, so I opened a new issue.

@hyrious
Copy link

hyrious commented Jan 4, 2023

This should be an issue in tsup instead of in esbuild. I just took a look at your demo repo. My previous comment works in running esbuild directly. Here's the build script so you can try putting it in your project and run it:

// make.js
var fix_react_jsx_plugin = {
  name: "fix-react-jsx",
  setup({ onResolve }) {
    onResolve({ filter: /./ }, (args) => {
      if (args.path === "react/jsx-runtime") {
        return { path: args.path + ".js", external: true };
      }
    });
  },
};

require("esbuild").build({
  entryPoints: ["src/index.tsx"],
  external: ["react"],
  bundle: true,
  plugins: [fix_react_jsx_plugin],
  outdir: "dist",
  format: "esm",
});

Run node make.js yields dist/index.js to have react/jsx-runtime.js.


So what's wrong in tsup? It turns out that it has an external plugin which automatically externalizes peer dependencies and it is put before any user's custom plugin.

esbuildPlugins = [
	...,
	externalPlugin, // marks 'react/*' as external
	...userPlugins,
]

That's why you have no chance to modify the external path which was already handled in tsup.


As a workaround instead of dropping tsup, you can run esbuild on the final file with bundle enabled but externalize all paths in onResolve callbacks, like so:

var input = 'dist/index.mjs'
require('esbuild').build({
	entryPoints: [input],
	bundle: true,
	format: input.endsWith('.mjs') ? 'esm' : 'cjs',
	plugins: [{
	  name: "fix-react-jsx",
      setup({ onResolve }) {
        onResolve({ filter: /./ }, (args) => {
          if (args.kind === "entry-point") return null;
          let path = args.path;
          if (path === "react/jsx-runtime") path += ".js";
          return { path, external: true };
        });
      }
	}],
	allowOverwrite: true,
	outfile: input,
})

@nettybun
Copy link

nettybun commented Jan 4, 2023

Don't let perfection get in the way of good enough - a simple find and replace should be enough, without involving the AST. Yes it's a hack, but it will let you move on to real work.

output = output.replace(/from "react/jsx-runtime";/g, `from "react/jsx-runtime.js";`);

@evanw
Copy link
Owner

evanw commented Jan 4, 2023

I didn't reply on that issue because my original comment still stands. You are trying to go against the conventions that the community uses, which is going to create extra work for you and for other people who try to use your code. I still recommend sticking with the way jsxImportSource is supposed to work instead of trying to change it.

I'm closing this issue because your question has been answered, both in that thread and in the above comments.

@evanw evanw closed this as not planned Won't fix, can't repro, duplicate, stale Jan 4, 2023
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