-
-
Notifications
You must be signed in to change notification settings - Fork 593
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
Alias plugin is not compatible with rollup-plugin-alias #27
Comments
@thgh The tone of this one could be a little more friendly. The change was debated and discussed by @thiscantbeserious, @Andarist, and @lukastaegert - hardly one user. I'd argue it was up for a reasonable amount of time for discussion as well. From what I understand, the API changed, but the functionality remains. |
Edited out unrelevant non-constructive comment from my side. Any reason why you don't stick with V1 if you don't need the functionality that V2 brings? |
The reason for a change was simple - we believe that this is a better API than what was present in v1. It was hardly for "no apparent reason" - v2 was intentionally published as a major version and thus breaking changes are expected. |
@thgh has done a lot for Rollup over the years, so I highly doubt the concerns are conveyed with ill intent. I'd ask everyone commenting take that into consideration. I didn't tag everyone with the intent to cause a pile-on, rather to further discuss the changes that were made. I'd be interested to know if there's middle ground here. |
First of all, sorry for the tone of the issue. I think it's fantastic that you all take the initiative to maintain many plugins. Also, I'm definitely not trying to point fingers (even though I see now that it looked that way). It's more that I'm concerned about the stability of the Rollup plugin ecosystem. Another concern is that tons of developers have struggled to configure webpack. Requiring them to learn another plugin configuration brings mental overhead. The effort to support both syntaxes in code is trivial, so I think the tradeoff leans in that direction. I hope the PR brings us closer to middle ground ;-) Note: I'm currently sticking to v1, but it's marked as deprecated (which triggered me to post this issue). |
* feat(alias) add backwards compatible mapping options Resolve rollup/plugins#27 * chore: migrate rollup-plugin-node-resolve (WIP) * chore: move types * chore: update lib, get deprecated tests working * chore: reorg tests * test: working on tests * test: all tests but one passing * test: fix last test, clean up * chore: fix linting * test: use snapshots to diagnose failing test * chore: add nested node_modules in fixtures * test: fix type tests * chore: update meta * chore: review comments * chore: update snapshots * Update packages/node-resolve/CHANGELOG.md Co-Authored-By: Huáng Jùnliàng <jlhwung@gmail.com> * chore: checkout alias/src from master * chore: checkout alias from master * chore: update pnpm lock * fix: use right pluginutils dep
How Do We Reproduce?
Add
alias({ src: __dirname + '/src' })
to rollup pluginsExpected Behavior
import 'src/util.js'
worksActual Behavior
The API change in plugin-alias@2 breaks the old behaviour for no apparent reason. It's perfectly possible to add a special
entries
option. It's sad that a project with 20k+ downloads/week can just break dependency upgrades because one developer feels like it.Breaking PR: rollup/rollup-plugin-alias#53
Previous issue: rollup/rollup-plugin-alias#58
Still incompatible PR: rollup/rollup-plugin-alias#61
(sorry for repeating this issue)
The text was updated successfully, but these errors were encountered: