-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix dependency issue when using @rollup/plugin-commonjs #5797
Conversation
@DSergiu: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
Is there an easy way to add "build with rollup" to our CI so that we don't reintroduce issues like this? It would be nice to only have one import of the symbol in our code base. How about moving the import to |
👍 Sure thing. I did extract gql into To replicate the issue, import gql from index.ts TypeError: (0 , __1.gql) is not a function |
sourcemapExcludeSources: false, | ||
}; | ||
const bundle = await rollup.rollup({ | ||
input: path.resolve(__dirname, '..', '..', 'dist', 'index.js'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth noting that I believe this works because our npm pretest
script builds the package ahead of time.
Thanks, this looks good! We'll do our best to keep rollup working. |
@rollup/plugin-commonjs has a hard time with this dependency and results in "undefined error" since
gql
is used before it is defined in the bundled code.One can argue that the fix should be in the rollup plugin, but it is a tricky fix, and historically there were many attempts to solve it with no complete success.