search

rollup / plugins

🍣 The one-stop shop for official Rollup plugins
MIT License
3.57k stars 568 forks source link

Modernize @rollup/plugin-terser to module #1659

Closed vdegenne closed 6 months ago

vdegenne commented 6 months ago

Rollup Plugin Name: @rollup/plugin-terser

This PR contains:

Are tests included?

Breaking Changes?

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

tada5hi commented 6 months ago

I would like to postpone this until we migrate the whole monorepo to esm.

shellscape commented 6 months ago

Let's not close PRs until we've had a chance to review.

shellscape commented 6 months ago

@vdegenne could you please explain your reasons for this change? We export both ESM and CJS for each package, which should make usage seamless. Is there a problem that you're trying to solve with this change?

vdegenne commented 6 months ago

@vdegenne could you please explain your reasons for this change? We export both ESM and CJS for each package, which should make usage seamless. Is there a problem that you're trying to solve with this change?

Sorry I think it's a misunderstanding, I was trying to use @rollup/plugin-terser in rollup.config.TS and I had an error, adding "type": "module" manually into the package.json solved the issue... ...until I realized I was using "module": "NodeNext" in my ts config. I changed the value to Node and the error was gone. I see this package is still providing CommonJS so I guess you are not going to accept this change any time soon.

shellscape commented 6 months ago

OK thanks for explaining. Yeah unfortunately we won't be moving to only ES exports soon.