Feature: default exports as an array #40
Conversation
| @@ -12,7 +12,7 @@ const entry = '\0rollup-plugin-multi-entry:entry-point'; | |||
| export default function multiEntry(config: ?Config = null) { | |||
| let include = []; | |||
| let exclude = []; | |||
| let exporter = path => `export * from ${JSON.stringify(path)};`; | |||
| let exporter = buildNamedExports; | |||
There was a problem hiding this comment.
the exporter now operates on an array of paths rather than just a single path
|
The CI situation for Node 6 is failing for some reason. Doesn't seem related to my stuff. |
|
@arshaw I fixed the issue on master and rebased. |
|
Can we guarantee the ordering of the array? Do we know that |
|
Thanks @eventualbuddha . It looks like matched mostly defers to node-glob, and I doesn't document anything nor expose anything about ordering. However, I would say this is also an issue for the default behavior of We can decide to force an order, via Array#sort for example, but I think that should be for the default behavior of this plugin as well. Please let me know if you'd like me to write code to force an order for either my new feature, for the plugin as a whole, or not at all. |
|
Sorry for taking so long to get back to you. I think doing |
|
Okay, would you like me to do the sorting regardless of what kind of |
|
Yep, I think that’s best. |
|
Sorry for the delay! I've made it so that the ordering of the array results as well as the execution-order of the name-exported-modules is now sorted and deterministic. BTW, here's more supporting evidence that the matched lib doesn't return in a particular order: jonschlinkert/matched#10 Let me know if there's anything else I should do! PS- test in the node 6 environment are failing. chokes on the es6 |
test/test.js
Outdated
| include: ['test/fixtures/default-1.js', 'test/fixtures/default-0.js'], | ||
| exports: 'array' | ||
| }).then(moduleExports => { | ||
| deepStrictEqual(moduleExports, [0, 1]); |
There was a problem hiding this comment.
This doesn't feel right to me. I think what should happen is that globs get expanded in place in sorted order, but everything else remains as it was. So in this case as there are no globs, each entry stays where it is in the list. What do you think?
There was a problem hiding this comment.
I agree. I changed it to honor your recommendation.
I originally implemented it how I implemented to be defensive... if you upgrade to the latest matched (4.0.0), the order becomes unpredictable. Solution: just don't upgrade. Stay at ^1.0.2.
Sound okay?
|
@eventualbuddha @arshaw I'd like to begin the process of moving this plugin to our new monorepo. To do that we have to get the outstanding PRs resolved. Any chance you all can wrap this one up this week? |
|
sure. the ball is in @eventualbuddha 's court to review my latest changes |
|
I'm unlikely to get to this as I am not a user of this package anymore. |
|
@lukastaegert I'm consumed with the migration of a few plugins at the moment. Could you take a look at this one? |
|
After discussing at length with other plugin contributors we've come to the conclusion that your use case is absolutely valid, but the likelihood that this would be a widely used feature is pretty slim. The good news is that none of your work has been wasted at all, and this can easily be made a separate third party plugin. We feel that'd be the best route to get the function you're after. Because of that, were going to close the PR, but please don't take that as a rejection of your work. Thank you for your work on this PR and for keeping with it through the delays. Consider me at your service for any assistance in getting this PR turned into a plugin, I'm happy to help in any way I can. |
I needed to make a new module that took a wildcard-generated list of other modules, take their default exports, and export them as an array in the resulting bundled module.
To make the usecase more concrete, in my project FullCalendar, I needed to take all the locale files and turn them into a "locales-all.js" file. Does that make sense?
Your module was a great starting point for this! I contemplated simply forking your code and making a new plugin, but ultimately decided it'd be best as a new feature. I've added a
exports: 'array'option. I've also added a test and I've update the README.I also had to refactor the tests a bit. The previous tests were doing string comparisons on the generated code, which is rather brittle of Rollup decides to mangle variable names in the future. While not that big of a deal for existing features, it was absolutely necessary for my new feature. Thus, the tests now primarily try to execute the resulting module by dumping it to the temp file and requiring it, and looking at the export object. Fingers crossed you are okay with this.
Please let me know what you think! I am open to any feedback.