Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make entry point consistently CJS instead of ESM and CJS combined #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arackaf
Copy link

@arackaf arackaf commented Mar 3, 2018

Sorry for all the GitHub notifications. Your entry point was a mix of ESM for import, and CJS for export. This causes errors if people alias webpack to just consume the entry point directly. Making the entry point consistently CJS fixes this problem, without introducing any breaking behavior.

@rse
Copy link
Owner

rse commented Mar 3, 2018

I'm using ESM on imports in nearly all libraries and intentionally the good old CJS export for compatibility reasons and I've not seen any trouble with Webpack until now. Can you be more specific when and why Webpack breaks when using graphql-query-compress?

@arackaf
Copy link
Author

arackaf commented Mar 3, 2018

Sure, basically adding this

"graphql-query-compress": "graphql-query-compress/src/graphql-query-compress"

causes a TypeError at runtime

on module.exports = compactGraphQLQuery since of course module doesn't exist.

webpack sees the ESM import at the top of the file, and processes it, and assumes the file is ESM. It then ignores the call to module.exports.

@arackaf
Copy link
Author

arackaf commented Mar 3, 2018

Of course the secondary workaround would be to point it at the Node build, but I really think you should consider making these modules consistent. I'd love all ESM, but that would be a breaking change, and so all CJS would in this case be as good, with no breakage anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants