search

jaredpalmer / tsdx

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

Upgrade @rollup/plugin-commonjs version #975

Closed bartsidee closed 3 years ago

bartsidee commented 3 years ago

We are working on a project and found a bug in the commonjs rollup plugin that prevents us from compiling the project properly for the commonjs module format which is failing on a nested symbol-observable depedency.

This is related to the issue reported her: https://github.com/rollup/plugins/issues/304

And was fixed in plugin release 12.0.0 https://github.com/rollup/plugins/blob/master/packages/commonjs/CHANGELOG.md#v1200

This PR proposes to upgrade the @rollup/plugin-commonjs module from 11.1.0 to 12.0.0 to address this.

Note: the latest version of @rollup/plugin-commonjs is 17.1.0, but I'm not fully able to access the potential impact

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/lidjn296x
✅ Preview: https://tsdx-git-fork-bartsidee-feature-commonjs-fix.formium.vercel.app

agilgur5 commented 3 years ago

Thanks for the contribution @bartsidee .

I know @rollup/plugin-commonjs has many bugs and their CHANGELOG is far from the most readable, so I understand that struggle. The bugs break a few other things, such as #759 which is similarly caused by https://github.com/rollup/plugins/issues/304, among other issues.

Unfortunately this is a breaking change and would have to go into a breaking release as such. There is also already an existing PR ready for the next breaking release: #889 .

So I'll have to close this as a duplicate as such.

In the meantime, you're welcome to update your resolutions to bump to the version you need (at your own risk).