doesdev / rollup-plugin-analyzer

Mad metrics for your rollup bundles, know all the things
http://rollup-plugin-analyzer.doesdev.com
MIT License
242 stars 4 forks source link

Default export #6

Closed timdp closed 5 years ago

timdp commented 6 years ago

Sorry to be a pain with all these feature requests. :-)

I wrote rollup-load-plugins a while back to avoid having to do the import xyz from 'rollup-plugin-xyz' dance for every plugin. That code expects a default export, which seems to be the convention in Rollup land.

As you're well aware, rollup-plugin-analyzer exports { plugin } instead. In code that uses both, I therefore have to update the alias manually after calling loadPlugins(), which is of course a bit unsightly.

I see some other exports in the source code but those aren't documented in the readme. I don't suppose you'd be willing to use a default export for the plugin?

Alternatively, I'd be happy to agree on a good way for rollup-load-plugins to recognize the { plugin } export from your code. I'm a bit reluctant to inspect the exports object for every plugin though.

doesdev commented 6 years ago

Not a pain at all, it's always helpful to have feedback on improving things. Conventionally speaking it would make allot more sense than the named exports if only the main export is being used.

Let me think on this a bit. One of the non-documented exports may be in use as I added it as a requested item. I'll evaluate after work and see what makes the most sense to do.

timdp commented 6 years ago

You could:

My workaround is in place for now so the urgency from my side is low. Just floating some ideas based on prior experience.

doesdev commented 6 years ago

TL;DR

Unless a compelling reason to keep named exports arises I'll plan to export the plugin function as default on the next semver major release. Any additional functions will become properties of the exported function and can be addressed that way. However, I think a semver major bump will only be appropriate if adding some more benefits like visualizations. So that might be the opportunity to do it. Let me know your thoughts.

Long version

The named exports emerged due to an (admittedly poor) attempt to rectify previous questionable design choices. Initially this project was a standalone module (rollup-analyzer) rather than a plugin. That of itself was less than ideal with Rollup providing such rich options for plugins.

Later I added a plugin and config module. However, the plugin module did not follow Rollup naming conventions. The plugin version and primary modules were then competing with each other without any clear reasons as to why the non-plugin module should even exist. However, it still was getting the most usage.

As such I deprecated all 3 of the previous modules, preferring instead to introduce a properly named module along with a more specialized config-only module. At that point what I should have done was a default export with the additional functions that I export as properties of the default export (or not exported them altogether). Instead I included all of the previously named exports of the core module in the plugin module and here we be.

It wasn't too long ago that I made those deprecations in favor of this module. I imagine that move alone was quite frustrating to users. It's not one I made lightly, but it did seem best to correct course at that time.

All of that is to say, I want to avoid further frustrating users with another API change or reverting to an additional module. As you pointed out doing named and default would be poor design, against the rules, and can't be expected to work long-term as Node converges on standardizing their ES Module implementation.

So, I think at this point waiting for a good opportunity for a semver major bump that includes more benefits than just the export naming change would be for the best. Perhaps the visualizations will be just the opportunity to do so.

I'll leave this open until then.

doesdev commented 5 years ago

Okay, in the end I went with the poor design option ;)

I guess I was mistaken in thinking the ECMA specs don't allow for both (perhaps I had simply wrongly assumed that). Since it is supported (even though it is poor design) I went ahead and did both a default and named exports. That is available in 3.0.0.