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

"tsup-node" doesn't respect "noExternal" configuration #718

Closed
eric-burel opened this issue Sep 16, 2022 · 4 comments · Fixed by #720
Closed

"tsup-node" doesn't respect "noExternal" configuration #718

eric-burel opened this issue Sep 16, 2022 · 4 comments · Fixed by #720
Labels

Comments

@eric-burel
Copy link
Contributor

eric-burel commented Sep 16, 2022

Hi,

I've been using "tsup-node to exclude all packages" with great pleasure to build some scripts while excluding all node_modules.

I am refactoring a monorepo, and I am in the special case where I have internal NPM packages that expose pure .ts code.
Therefore, I need those specific packages to be included in the build, despite being listed in the package.json.

The use case is pretty close to what's described for next-transpile-modules: https://www.npmjs.com/package/next-transpile-modules "code transpilation from local packages (think: a monorepo with a styleguide package)".

What's the equivalent in Tsup? How can we forcibly reinclude some packages while using tsup-node?

Here is an example: Devographics/Monorepo#117, the scripts are built in the "surveyform" directory, alongside the Next app. Package "core-models" lives in the shared folder, you can see how the package.json let's us expose pure TS files. I need this specific package to be bundled despite it being listed in package.json as a dependency.

@koshic
Copy link

koshic commented Sep 18, 2022

@eric-burel is #538 can help?

@eric-burel
Copy link
Contributor Author

eric-burel commented Sep 19, 2022

@VladimirGrenaderov great tip thank you, however I don't really see the effect of this command, the built file is completely similar. The option is read (if I put a wrong string instead of an array it fails) but has no effect.

Example config:

const commonConfig = {
  clean: true,
  splitting: false,
  // Skip until .d.ts.map is also supported https://github.com/egoist/tsup/issues/564
  // dts: true,
  sourcemap: true,
  tsconfig: path.resolve(__dirname, "./tsconfig.tsup.json"),
  outDir: "scripts/dist",
  name: "",
  platform: "node" as const,
  target: "node14",
  noExternal: [/^@devographics($|\/)/],
  esbuildPlugins: [yamlPlugin({})],
};

Note that this is with tsup-node not tsup as I build a Node script. With tsup it works as intended. So this affects only tsup-node.

I am digging the code and it seems that the only thing tsup-node does is using the skipNodeModulesBundle: true option, which is a tad too aggressive as it also skips packages that are explicitely listed as not being externals.

The code in src/esbuild/external.ts seems to confirm that the "external" computation logic doesn't use the "noExternal" option when "skipNodeModulesBundle" is true. I think it should, I'll give a shot at a pull request.

@eric-burel eric-burel changed the title Exclude all packages...but one Exclude all packages...but one with "tsup-node" command Sep 19, 2022
@eric-burel eric-burel changed the title Exclude all packages...but one with "tsup-node" command "tsup-node" doesn't respect "noExternal" configuration Sep 19, 2022
@eric-burel
Copy link
Contributor Author

eric-burel commented Sep 20, 2022

#720 fixes but if someone needs it right now, they can use this config with tsup in replacement to tsup-node:

// whatever you need to bundle (here my shared "@devographics" internal packages and my path alias "~")
  noExternal: [/^@devographics($|\/)/, /^~/],
// everything else (this regexp is taken from the code used when tsup-node or skipNodeModulesBundle option are used)
  external: [/^[^.\/]|^\.[^.\/]|^\.\.[^\/]/],

@github-actions
Copy link

🎉 This issue has been resolved in version 6.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

2 participants