Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Relay Support in Rust Compiler #33240
Relay Support in Rust Compiler #33240
Changes from 9 commits
2aaa426
f794fcf
6c966d2
8ee3e6c
46e2534
66e6859
eb88c71
4ef5e23
90d3bce
718929c
6d03ee9
523bc25
61826a4
3fe7119
eb7daa1
4f75ae5
46f46f2
400def3
b6d924f
d419ba0
3830a8d
5fa72cf
cd6ed9c
f7cfd1c
fee2e11
2b53fa1
5612526
244a4da
ba4e846
8ba0998
8782671
8343e17
686ff01
27057e5
acea05f
85ac72c
7d35873
d24ae71
5f3f298
aa0f676
4a4a5b6
f6cc9d7
fffa3e1
b5f89bc
1682ac3
464dd97
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
not directly related to this specific PR, but I wonder how this can be implemented here.
Currently, our
babel
plugin fundamentally does two things: 1) adds require('...') calls instead ofgraphql
tags. and 2) warns if the text operation/fragment is changed, but was not recompile, and displays a runtime warning: https://github.com/facebook/relay/blob/main/packages/babel-plugin-relay/compileGraphQLTag.js#L128Not sure if there is JS runtime part in next.js that can also handle that second part.
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.
@alunyov Can you explain the use case for 2? Might be able to get away with less complexity if we don't add it.
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.
@tbezman Sure! A developer changed the fragment or a query (added/changed field, etc), but didn't run the
relay-compiler
. When they refresh the page, the generated artifact for this file (.graphql.js file) won't contain the changes, this sometimes may lead to confusing situations, when developers may not see changes they are expecting on the screen.To reduce the confusion, In the babel plugin we're comping the
md5
hash of the text insidegraphql
tag. And we're comparing this hash to thehash
in the generated artifact (created by the compiler), if these hashes are different - we're showing a warning in the browser's console. When developers debug the outdated views, they may see the reason in the console.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.
That makes a ton of sense. @alunyov Is there somewhere in the relay source where y'all export a function to hash a query / mutation / fragment / subscription? Hoping we can avoid duplicating functionality here.
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.
Also, maybe we can address this in a follow up PR. I'm helping a team migrate to Relay / GraphQL this week and want to keep them on the Rust compiler.
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.
@tbezman sure, totally fine to do this in a follow-up.
This is what we're using to create these
source_hashes
:https://github.com/facebook/relay/blob/14550c8bd80e53369852db66d7681f13d6dd5f84/compiler/crates/relay-compiler/src/build_project/build_ir.rs#L33-L36