search

jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.24k stars 508 forks source link

Update to Rollup 2 #889

Open ludofischer opened 3 years ago

ludofischer commented 3 years ago

(maintainer edit to add tags as this relates to a lot of issues):

Tags

Resolves some issues:

Closes out some related / now duplicative PRs:

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/formium/tsdx/g1z5a4eq5
âś… Preview: https://tsdx-git-fork-ludovicofischer-rollup2.formium.vercel.app

ludofischer commented 3 years ago

Looks like the MacOS failure is anetwork problem on the test machine. I believed this would be harder, am I missing something? the only change to the code itself is that the terser plugin does not accept a sourcemap option anymore, instead it infers it from the rollup options (see https://github.com/TrySound/rollup-plugin-terser/releases/tag/v6.0.0)

agilgur5 commented 3 years ago

I believed this would be harder, am I missing something?

Rollup has not been upgraded for lack of effort, it's because it is a very breaking change, likely affecting a good portion of tsdx.config.js users. See https://github.com/formium/tsdx/issues/821#issuecomment-678745312. As mentioned there, previously it was also not well supported, and I believe there are still issues with TS 3.8+ as I mentioned in that comment and where it links to: https://github.com/formium/tsdx/pull/679#issuecomment-631478298 It also drops Node 8 support, as does plugin-terser. TSDX v0.14.0 was just released a week ago that now requires Node 10+ -- Node 8 was still supported until a week ago.

I also wrote in the release notes of v0.14.0:

I also wanted to push Rollup 2, TS 4.0, Prettier 2, and ESLint upgrades into this, but the breaking changes in the changelog started getting too big, so I decided to wait a bit to split those changes across more releases to not throw too much breakage at users at once.

I'm not planning on releasing Rollup 2 at least for a few weeks to spread out aforementioned breakage, and I was planning to batch that in with v0.15.0's changes, which also are only breaking to a subset of tsdx.config.js users. But as TS 4, Prettier 2, and ESLint were left out of v0.14.0 intentionally, I was thinking of shipping those as v0.15.0 and pushing what's currently slated as that until v0.16.0. Those could be interchanged though.

ludofischer commented 3 years ago

Yes, as I was making the changes I realized maintaining a compiler tool like this must be horribly complicated as it’s hard to check if and how you’re breaking other projects. It’s even harder as you’re not really in control of what upstream dependencies decide to release.

agilgur5 commented 3 years ago

@ludovicofischer indeed. I appreciate the acknowledgement and understanding.

Breaking changes really require a lot of thought and planning of maintainers and lot of factors in TSDX don't make that easier. That's one of the reasons why the Unix methodology of small modules doing one thing well is a good one to follow -- the smaller the API surface, the less potential for breakage there is, and the more known what that breakage looks like. The scoping problem becomes larger with the introduction of any new API, which is why those decisions should really be made quite carefully. Some maintainers can widen out APIs too quickly and also break APIs unknowingly frequently; the former makes the latter an easier mistake to make.

Being a compiler tool as you say increases the scope and potential impact of breakage to be quite large; it can literally break the correctness of your built code (see also the problems with async-to-promises and the recent switch to regenerator in #795). tsdx.config.js opens up basically the whole Rollup API as a TSDX API that can be broken. That's why I discourage it and say to only use if necessary as an escape hatch, per the warning I added in #400 (when I was still a contributor). It's very difficult to promise that we'll know if anything breaks that, and nearly any plugin change or minor change could be considered breaking, so that warning is also a warning that unforeseen breakage is to be expected with tsdx.config.js. This specific PR's breakage is quite foreseen though, so I think good faith should be made to plan it as a breaking change and notify users. Being an all-in-one tool also further increases this scope. A breaking change to any one of tsdx build, tsdx test, tsdx lint, tsdx create, etc also mean that I have to release a breaking change for the entirety of TSDX since they're all currently combined as one big API of TSDX.

635 is one of my approaches to improve the tsdx.config.js problem (make maintainable and testable plugins) and #634 is an approach to start splitting out some of the API. I'd like to split out each command into a separate package as well, that way I can work on improving, say, @tsdx/test independently and release a breaking change there without affecting tsdx test until that is upgraded. Can then batch breaking changes together more easily and also allow users to opt-out or opt-in to different pieces of TSDX without requiring the whole thing. It can also allow for things like @tsdx/jest vs. @tsdx/mocha, etc.

ludofischer commented 3 years ago

Great! I’ve been happy to help.