Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Don't generate a default export for ES modules that don't have one #225

Closed
wants to merge 1 commit into from
Closed

Conversation

marijnh
Copy link
Contributor

@marijnh marijnh commented Sep 7, 2017

I think this might fix #224

marijnh added a commit to ProseMirror/website that referenced this pull request Sep 7, 2017
@marijnh
Copy link
Contributor Author

marijnh commented Sep 7, 2017

Oh, failing tests. Trying to run them locally, both current master and with my patch, I get an error like

/home/marijn/tmp/rollup-plugin-commonjs/test/test.js:50
async function executeBundle ( bundle, { context, exports } = {} ) {
      ^^^^^^^^
SyntaxError: Unexpected token function

Which seems like an acorn configuration issue? Will try to guess what's going wrong and rely on CI to test.

@marijnh
Copy link
Contributor Author

marijnh commented Sep 7, 2017

Fixed one of the tests, but the remaining failure is more involved -- it breaks because hasDefaultExport is (incorrectly) false for the sample module.exports = { default: 'foobar', __esModule: true }. Should we try to make it smarter, or is it, since this can never be determined with full accuracy, better to try and find an approach that doesn't break when the detection goes wrong?

@marijnh
Copy link
Contributor Author

marijnh commented Sep 7, 2017

Oh, I finally got why I couldn't run the tests -- you're directly using async/await and I was using a pre-7.4 node. That only took me an hour to notice. I should go to bed.

@marijnh
Copy link
Contributor Author

marijnh commented Sep 7, 2017

Hm, this solution is not really coherent—it's mixing up two interpretations of what default export means (CJS module.exports vs ES export default), and it only fixed my use case by incident. Closing this PR, let's continue in #224

@marijnh marijnh closed this Sep 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default export added on *-imported CJS module causes problems
1 participant