Fix CLI and dynamic import workflow bugs

[guybedford]

In the process of putting together some example material these came up that were missed with some of the changes.

[guybedford]

The example workflow I found these on was for rollup/rollupjs.org#110.

[lukastaegert]

As you can see here with this change, the code will be broken (you would need to write dep2.default.mult(num) to make it work).

[guybedford]

This is no longer happening now.

[lukastaegert]

What is the scenario you hope to fix here? Test? As you can see with the test below, this makes named exports no longer work (and again shows that we probably need functional tests). My feeling is that the correct approach would be more complicated.

[guybedford]

Great catch. Ok so the rule is this:

• When the dynamic import is an external module, we need to treat it as { default: module } as that is how interop works when importing CommonJS from ES modules.
• When the dynamic import is a known module from our current bundle operation, we treat it as a full module object, as it's effectively within our private interface for bundling.

I've added the adjustments to make these cases work out now.

[lukastaegert]

This looks good now (though I did not run all cases myself) 👍

[lukastaegert]

Even though I just merged this, there is one minor thing I just noticed: If inputOptions is an array of length one and you specify neither output.dir nor output.file, generate will return undefined for the code and rollup will fail. I will revert this particular change for this release for now as the previous behaviour also better corresponded to this check

rollup/src/rollup/index.ts

Line 235 in c7981c4

const codeSplitting = inputOptions.experimentalCodeSplitting && inputOptions.input instanceof Array;

and we'll need to find a better solution here

[guybedford]

Thanks for spotting this - assuming the example workflow still runs ok sounds sensible to me.

…

On Fri, 26 Jan 2018 at 08:55, Lukas Taegert ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In bin/src/run/build.ts <#1907 (comment)>: > @@ -10,7 +10,7 @@ import { BatchWarnings } from './batchWarnings'; import { SourceMap } from 'magic-string'; export default function build (inputOptions: InputOptions, outputOptions: OutputOptions[], warnings: BatchWarnings, silent = false) { - const useStdout = outputOptions.length === 1 && !outputOptions[0].file && inputOptions.input instanceof Array === false; + const useStdout = outputOptions.length === 1 && !outputOptions[0].file && !outputOptions[0].dir; Even though I just merged this, there is one minor thing I just noticed: If inputOptions is an array of length one and you specify neither output.dir nor output.file, generate will return undefined for the code and rollup will fail. I will revert this particular change for this release for now as the previous behaviour also better corresponded to this check https://github.com/rollup/rollup/blob/c7981c447677d06a3646cc26299eb5dc357998db/src/rollup/index.ts#L235 and we'll need to find a better solution here — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1907 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAkiyr2FqIslveRvhKB52WfEHiPMXAV7ks5tOXb-gaJpZM4Rp0d8> .