[commonjs] named export missing with v13 + dynamic imports

[lazka]

• Rollup Plugin Name: @rollup/plugin-commonjs
• Rollup Plugin Version: 13.0.0
• Rollup Version: 2.18.0
• Operating System (or Browser): Firefox 77
• Node Version: v12.18.1

How Do We Reproduce?

I'm including fabricjs via dynamic imports and the update from v12 to v13 removed the named export. Static imports still work though.

async function foo() {
    const x = (await import('fabric')).fabric;
    console.log(x); // this is undefined but wasn't with 12.0.0
}

foo();

This works though

import {fabric} from 'fabric';

I've created a small reproducer: https://github.com/lazka/rollup-bug-dynamic-import-exports

Expected Behavior

Named exports work the same with dynamic imports as with non-dynamic ones

Actual Behavior

They are missing in the dynamic case.

[shellscape]

Probably similar to #451

[lazka]

The weird thing is if I add a dummy static import at the top the dynamic one works again and provides the named export.

[lukastaegert]

I think it is different from #451 and might be actually a bug in Rollup itself, specifically in how synthetic named exports interact with dynamic imports. Will need to investigate.

[lukastaegert]

There will be a fix in Rollup, see rollup/rollup#3648. I will put it into the next patch release. This should fix this bug without necessary changes to this plugin.

[lukastaegert]

Fixed in Rollup@2.18.1

[lazka]

Thank you!

edit: also confirming that things work again with 2.18.1 :)

[desmap]

@lukastaegert it seems still to be broken with 2.25

Pls check the demo repo: https://github.com/desmap/project-refs

When you run it you get:

const { ObjectType, Field } = process.env.SHIM
        ^

TypeError: Cannot destructure property 'ObjectType' of '(intermediate value)(intermediate value)(intermediate value)' as it is undefined.

The dynamic import is in p1/index.ts:

const { ObjectType, Field } = process.env.SHIM
  ? await import('type-graphql/dist/browser-shim.js')
  : await import('type-graphql')

console.log(Field)

export default 'Hello World!'

You need to run it with NODE_OPTIONS="--experimental-top-level-await" npm run dev

It works with a static import ... from

CC @lazka

[lazka]

I'd suggest you open a new issue for this.

[desmap]

@lazka I can do this but why a second issue about the exact same thing?

It's again about named export[s] missing with v13 + dynamic imports.. Maybe it doesn't work because I used a top-level await but this shouldn't be the reason (hopefully)... whatever it's not working.

[desmap]

@lukastaegert let me know if you want a new issue about the my post above

[shellscape]

(Please remember to use the "Edit" feature for successive replies to help cut down on notification noise.)

[lukastaegert]

Yes please, because even though the symptom is the same, the cause obviously isn't. This helps me with book-keeping and adds visibility so that others notice this issue and can have a look as well.

[lazka]

(For completeness sake I've tested my reproducer and it still works correctly with the latest versions of everything)

[desmap]

@shellscape

(Please remember to use the "Edit" feature for successive replies to help cut down on notification noise.)

In this case it was on purpose. Github does not notify users added in an edit. Besides, pls be aware that your post creates unnecessary noise as well and is not welcoming.

[desmap]

@lazka

(For completeness sake I've tested my reproducer and it still works correctly with the latest versions of everything)

Thanks for checking it out again. Did you test it with a top-level await?

[lazka]

Thanks for checking it out again. Did you test it with a top-level await?

No. I just want to rule out that this is a regression.

[shellscape]

@desmap I asked politely. Please heed the requests of maintainers. Locking as resolved to prevent further superfluous comments. If this becomes an issue for anyone else in the future, please create a new issue.