tsx
tsx copied to clipboard
privatenumber
Stop minifying the dist files sent to NPM
Precheck
- [X] I searched existing issues before opening this one to avoid duplicates
- [X] I understand tsx aims for TypeScript parity and will not support arbitrary Node.js enhancements
Feature request
I think tsx should be served with non-minified files.
Motivations
It's almost impossible to use patch-package (or anything equivalent) when the package is minified, and tsx is not meant to be run on the browser (where KBs actually make a difference), so it just makes it harder for those of us who need to adjust its behavior while it doesn't provide any benefit (at least I can't see any). Even if the size was relevant, users would already have a bundler in their project to do the minification.
Alternatives
There's no workaround besides trying to deal with the minified code when patching.
Additional context
No response
Contributions
- [X] I plan to open a pull request for this issue
- [ ] I plan to make a financial contribution to this project
My project is using [email protected] and @esbuild-kit/[email protected]. I had to patch the latter a few months ago to make it work with the Firebase emulator and ESM. Now I'm stuck to those versions because when I upgrade to the latest version of tsx, it goes back to crashing the Firebase emulator with an obscure error and I can't easily put some console logs in node_modules/tsx to try to track down where exactly things are going wrong, so that's why I opened an issue asking to stop minifying instead of an issue about the specific error that I'm getting (because I wouldn't be able to provide any meaningful information to help track down and solve the problem). And debugging the minified code in node_modules/tsx would be so complicated that I have to keep the old version as it still works anyway.
It's hard to even remember what exactly the patch does because of the minification, which also makes very hard to explain to you. If it wasn't minified, I'd simply copy the whole patch file and you'd probably immediately understand what's going on just by looking at it.
These are the relevant snippets (in reality it's a single super long line in node_modules/@esbuild-kit/esm-loader/dist/index.js, but I extracted the parts separately to paste here):
-async function E(t,s,o){let a;for(const r of C)try{return await u(t+r,s,o,!0)}catch(n){if(a===void 0){const{message:c}=n;n.message=n.message.replace(`${r}'`,"'"),n.stack=n.stack.replace(c,n.message),a=n}}throw a}
+async function E(t,s,o){t=t.replace(/\.js$|\.jsx$/,"");let a;for(const r of C)try{return await u(t+r,s,o,!0)}catch(n){if(a===void 0){const{message:c}=n;n.message=n.message.replace(`${r}'`,"'"),n.stack=n.stack.replace(c,n.message),a=n}}throw a}
IIRC, the above makes it work with .js imports that actually should resolve to .ts or .tsx. I think this one was already fixed since my patch, but for some reason the fixed behavior still seems to crash the Firebase emulator.
-process.send&&process.send({type:"dependency",path:t})
+false&&process.send({type:"dependency",path:t})
-if(process.send&&process.send({type:"dependency",path:r}),r.endsWith(".json")||d.test(r))
+if(false&&process.send({type:"dependency",path:r}),r.endsWith(".json")||d.test(r))
And these remove a behavior that interferes with the Firebase emulator because it also uses process.send() for its own purposes and doesn't know what to do with the messages that come from tsx. I know I could also patch firebase-tools, but the patch applied to tsx was much simpler than if I was to patch firebase-tools.
Currently, there's a strong rationale for code minification. While I'm willing to consider stopping minification, we need compelling reasons to make that decision.
As an open-source maintainer who has invested numerous hours of free work for the community, I'm inclined to encourage contributions from others rather than making it easier to keep them as private patches.
If you want to propose halting minification, you'll need to offer more than just "I can't recall what I'm improving, and I'm not going to contribute it."
I'm going to close this, but if others can provide real use-cases that aren't about keeping improvements to yourself, feel free to share.
@privatenumber I think there might have been a bit of a misunderstanding regarding my intention. I want to contribute to enhancing tsx, and this is not just for my own benefit, but for other users as well. However, it's practically impossible to do that without being able to delve into node_modules and tinker around a bit. I need to do this to understand how the data flows and where the actual issue is occurring.
I'm asking you to make a trivial change to the build configuration that will give me the means to investigate further my obscure problem by my own and bring you a proposed solution instead of simply giving you insufficient clues and hoping you spend your time figuring it out.
It's not simply that "I can't recall what I'm improving, and I'm not going to contribute it". That's like the worst possible interpretation of what I said. I brought as much information as I could, but I really don't know anything beyond that because debugging obfuscated code is hard. It's possible this is not even a problem with tsx, but rather with firebase-tools. I'm literally trying to avoid wasting your time.
If you just want to debug tsx, why don't you make the changes in your own fork? If you're open to contributing, you'd need to set up a fork anyway.
Kind of strange to ask for a performance optimization be removed for everyone just to improve your debugging experience, when you can already do that for yourself right now by cloning the repo.
Alternatively, you can also run prettier on the minified tsx so it's easier to read.
@privatenumber This issue is about making it easier to contribute. If the code wasn't minified, there would be no need to fork. I'd just delve into node_modules, investigate where the issue is happening, come back to GitHub and either click the pencil icon to edit the file that needs to be edited or open an issue and handle the raw diffs so you would easily be able to adapt them to your liking.
I was assuming minification wouldn't affect the performance in a significant way. I guess I was wrong.
Anyway, just want to make it clear that I opened this issue with a good non-selfish intention (i.e. to make it easier for people — including but not limited to me — to contribute), and from the 👍 reactions it seems (as of now) 3 people (maybe 3 potential contributors?) agree with the idea.
This issue is about making it easier to contribute.
Contributors will have to fork the repo. You'll need to add tests to land the PR.
3 people (maybe 3 potential contributors?) agree with the idea.
Not a democracy. We need compelling reasons to make good decisions.
Contributors will have to fork the repo. You'll need to add tests to land the PR.
Alright.
Not a democracy. We need compelling reasons to make good decisions.
Fair enough. I thought other people seeming to agree would be an indicator that perhaps it may be compelling in the sense that it could drive more contributions (which is the whole goal of the idea), not in the simple sense that the majority chooses.
🚩 library authors should never minify dist files. this is a job for the app consuming the library.
To keep the conversation constructive, I'm going to hide comments that lack clear articulation and reasoning.