privatenumber / tsx

⚡️ TypeScript Execute | The easiest way to run TypeScript in Node.js
https://tsx.is
MIT License
9.8k stars 154 forks source link

feat(esbuild): make it possible to set `keepNames` with `TSX_ESBUILD_KEEP_NAMES` #659

Open Nemikolh opened 1 month ago

Nemikolh commented 1 month ago

Hey! Thanks for the really cool project :sparkles:

I've run into an issue with tsx where I'm using it with puppeteer and I get a crash because __names is not defined.

It essentially boils down to this code:

const result = await page.evaluate(() => {
  function func() {}
});

getting turned into this by esbuild under keepNames: true:

const result = await page.evaluate(() => {
  function func() {
  }
  __name(func, "func");
});

the issue is that __name is not defined in the page and so this errors.

See this playground

Given that I have no use for keepNames: true in my project, it would be nice if tsx allowed one to opt-out from keepNames: true via an environment variable.

Thus this PR. Let me know what you think! :smiley:

privatenumber commented 1 month ago

I've rejected this change before because I don't want to expose esbuild, especially if we swap it out in the future (which is becoming more likely with OXC developments). After thinking about it again, I realize I enabled keepNames because I also enabled minify to keep the cache small—so maybe we could turn these off if --no-cache is passed.

What might bother me is the difference in behavior between using cache and not using cache. I also considered creating a noop __name and injecting it globally, or using regex to replace __name inline, but that could get complicated if __name appears in user code. WDYT?