Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Terser plugin #1182

Closed
wants to merge 2 commits into from
Closed

Add Terser plugin #1182

wants to merge 2 commits into from

Conversation

nemzes
Copy link

@nemzes nemzes commented May 3, 2022

Rollup Plugin Name: terser

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

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

The currently-in-the-wild plugin for running Terser as part of a Rollup pipeline is rollup-plugin-terser. When using this with Yarn PnP the Terser process hangs (seems related to the use of jest-worker and this issue. rollup-plugin-terser seems abandoned, so I'm submitting a simpler plugin with no dependencies besides Terser itself. This runs fine with Yarn PnP.

@nemzes nemzes requested a review from shellscape as a code owner May 3, 2022 13:27
@shellscape
Copy link
Collaborator

Thanks for taking the time to put together a PR. Unfortunately this is one of those PRs that should really be discussed before hand. We even have an issue template for exactly that topic.

Back in 2019 we asked @TrySound if he wanted to merge his plugin into the repo here, and he stated then he'd prefer to hang onto it. One of the things we try to do as a project is really respect third party plugin authors. If there's a plugin out there in the wild that performs a function that we don't currently have here, then we respect that and pass on adding a new plugin, or a fork of a plugin, to the repo with identical functionality. Occasionally, we've added plugins where a community plugin was outright abandoned. rollup-plugin-terser still has 3,608,867 weekly downloads.

We can leave this open if you'd like to head to https://github.com/TrySound/rollup-plugin-terser to lobby @TrySound to move the plugin here if you'd like. Until @TrySound gives us the go-ahead, we won't be able to accept a new plugin to the repo that performs the same function. In the meantime, you're welcome to publish your work and add it to https://github.com/rollup/awesome

@nemzes
Copy link
Author

nemzes commented May 3, 2022

The reason I've added it here is that there hasn't been any movement on PRs or commits over at rollup-plugin-terser for a year and a half. But sure, just reject it if you want. 🤷

@shellscape
Copy link
Collaborator

That's not a helpful reply, and we don't encourage or support that kind of snark in this repo or community. We've given you a really thoughtful, well-explained response to your PR. We've also explained the level of respect and courtesy we pay to third party plugin authors, as they're who built this community. We're happy to leave this open if you can approach this with a healthy attitude and want to lobby @TrySound to move his plugin here. If you don't wish to do that, please accept our thanks for the effort of putting together the PR, and kindly close.

@nemzes
Copy link
Author

nemzes commented May 3, 2022

I think you're ascribing me attitudes I certainly don't carry.

It's hard to read eachother's demeanor over text, so I'll try to say this clearly: my motivation to submit the PR is that I faced an issue, identified the cause, worked on a fix, and then looked at the current repo and saw that there has been no movement there to several PRs and issues. Some of the unanswered issues there look very much like what I experienced. Since this is a fairly trivial thing, I wrapped it up as a plug-in for this repo since that seemed the most expedient & helpful solution in face of circumstances. It's not a "move the other plugin here" exercise.

I don't expect you to merge the PR simply because I submitted it; it's your repo and you decide whatever you want about contributions. I'll use this as my own plugin if not accepted.

Again, this is text and it's hard to read attitudes with text, so I don't want to judge yours too harshly — but my 2 cents is: try to come across as less patronising.

@shellscape
Copy link
Collaborator

If English is not your first language, you should be aware that this: "But sure, just reject it if you want. 🤷" is considered snark. It's condescending and it's rude. "But sure, ..." is never used in English as polite.

I'm sorry that you feel that thorough explanation of how we work with third party authors, and the consideration we give to new plugin additions here, was patronizing. Perhaps you failed to read it for what it was. In any event, we'll be closing this PR as the contribution is not a good match for this repo.

@shellscape shellscape closed this May 3, 2022
@rollup rollup locked as resolved and limited conversation to collaborators May 3, 2022
@nemzes nemzes deleted the add-terser-plugin branch May 3, 2022 17:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants