refactor(multi-entry): virtualised & entryFileName

[Bluefinger]

Rollup Plugin Name: multi-entry

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:

Prompted by #488
Fixes Windows related error and exposes new option for overriding defaults.
Idea for this PR from #506

Description

BREAKING CHANGES: Outputs a multi-entry file with different default name

Refactored multi-entry to use plugin-virtual for resolving/loading the multiple entries into a single entry file. Some tidy-up and adding a new option, entryFileName, to override the default entry filename.

First PR for rollup, feel free to provide feedback and further notes for work.

[shellscape]

This is an interesting take on the fix. Thanks for opening this. 🎉 We'll have a look at it shortly.

[shellscape]

Great work, thank you. We'll wait to merge until a few more eyes are on it. But LGTM

[pnevares]

Seems minor, but checking to make sure this isn't hiding a bug: you're always setting config.exports to true immediately before this condition.

[Bluefinger]

The ...conf at the end of the config definition will overwrite the default exports: true, since the documentation describes that you can pass a multi({ exports: false }) object into the plugin itself. Therefore, given that is a means by which you can configure this plugin, I have to initialise the prefix with the potential of being either an import/export type before I process the input. The style is the same as Object.assign({ exports: true }, conf)

That is how I understand the code when I came to have a bash at it.

[pnevares]

Whoops, of course you are right. Thanks!

[shellscape]

[cyclops24]

@shellscape @Bluefinger When you want to release this? As I checked the npm package version for multi-entry module is still 3.0.1 as before:

plugins/packages/multi-entry/package.json

Line 3 in e4d21ba

"version": "3.0.1",

[shellscape]

@cyclops24 please kindly refrain from asking about releases. we handle those as we're able to. rest assured it'll go out.

[cyclops24]

@Bluefinger @shellscape I checked this PR code in my local and it seems it's not working well with preserveModules: true. it still creates a new file with : in the name in this location _virtual\_virtual:multi-entry.js I also try to set the DEFAULT_OUTPUT props but nothing change. Also in preserveModules: true it creates a lot of file in folders with the name of multi-entry.js. For example:

build/component_1/multi-entry.js
build/component_2/multi-entry.js
build/component_3/multi-entry.js
build/component_4/multi-entry.js
...

Is it normal? IF not maybe we need to create another issue for this and fix these issues.

[cyclops24]

@Bluefinger @shellscape Did you read my message? I don't know why you merge and release this PR with a new bug on preserveModules: true mode here?
601f689

You want to fix windows issue but you break plugin in preserveModules: true mode.
Sorry, but when you prepare something as an official package the minimum thing is that read your community message when someone reports a breaking PR.
I think in this case unofficial packages are more reliable I also try to fork this multi-entry and continue on my fork.
Good Luck

[Bluefinger]

@cyclops24 Make an issue then? What is the expected behaviour that you wish to see with preserveModules: true, and whether it'll be its own breaking change in order to support that use case?

How a fix is applied also depends on what people expect in terms of behaviour and their own use cases. It might be that your case breaks anothers if we apply a fix there as well. Probably a good idea to discuss this in an appropriate issue as such.

[cyclops24]

@Bluefinger The issue is that this PR doesn't solve main problem and also cause another new bug:

1. The multi-entry plugin still create a file with : character like this: _virtual\_virtual:multi-entry.js
2. This PR break rollup functionality in preserveModules: true mode.

I informed you guys 2 days ago but I see rollup released this buggy version today. I told you everything here. No more time to spend on a simple : character change and discuss this.

For others, if anyone has an issue in Windows OS:
I created a multiEntry.js file in my project (based on previous version of multi-entry plugin) and change this line:

const entry = '\0rollup:plugin-multi-entry:entry-point';

To this:

const entry = 'rollup_plugin-multi-entry_entry-point';

Everything works without any problem also in preserveModules: true. I suggest to don't update this plugin until fixing this new known bug.

@Bluefinger I also suggest preparing new PR to fix the two above issues.

[Bluefinger]

@cyclops24 The issue with preserveModules: true then would apply for the virtual plugin as well. So we fix one plugin, but then still have another that will encounter the same issue on Windows. So it might not really be for multi-entry that needs fixing, but how virtual plugin interacts with preserveModules: true. And for default configurations, which is preserveModules: false, everything works fine.

This is kinda why you should open an issue, so that an actual working solution can be narrowed down and proposed.

[cyclops24]

@Bluefinger you tell in the PR description this note:

Prompted by #488
Fixes Windows related error and exposes new option for overriding defaults.

And also @shellscape closed the related issue: #488 that @aminya, me, and @frank-dspeed reported and marked this as a fixed issue and close the issue.
But the issue still exists there. Now you told me that we need another change on the virtual plugin to fix these 2 new issues so this PR, not a fix for that issue at all and totally a different thing. (refactor the plugin to use virtual plugin)

I really appreciate your contribution it's not a discussion between me and you. It's a lake of QA and attention to community messages that a simple : bug converted to a broken plugin.

@Bluefinger Also we don't need to open a new issue. The issue still exists there. We just need to reopen that:
#488

[shellscape]

Please open a new issue to track bugs related to this PR.

It's a lake of QA and attention to community messages that a simple : bug converted to a broken plugin.

I take exception to this language. We spend A LOT of FREE time trying to support the community. And forgive us for not catching every issue on every OS. 🙄 Please refrain from commentary like this in the future.