-
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
internal: fix inlineInvariant for cjs builds that require filenames with explicit extensions #3832
Conversation
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋 Supported commandsPlease post this commands in separate comments and only one per comment:
|
13d36e3
to
c7b366c
Compare
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.
Looks good!
We just need a comment documenting what we are testing in both files.
Otherwise ready to merge
8700dda
to
617a25a
Compare
Ready for review -- checks should pass once #3849 is merged. |
617a25a
to
dde36a1
Compare
Current build script rewrites invariant to avoid function call if invariant true.
However, the rewriting should take into account the import name as rewritten (by TS?):
Current output for cjs is:
graphql-js/execution/execute.js
Line 14 in ba59ddd
while the call is still:
graphql-js/execution/execute.js
Lines 723 to 724 in ba59ddd
yielding an error of the sort:
invariant is not defined