-
Notifications
You must be signed in to change notification settings - Fork 70
Add support for devOnlyAssignments #94
base: master
Are you sure you want to change the base?
Conversation
Oh! I didn’t even know of this. Great stuff, I guess we should also update the documentation upstream on the interface then.
I’m not sure if a Babel transform already exists that would strip property initializers from objects–it would certainly not be hard to create–but I do think that perhaps that could lead to a better situation where the person in charge of builds can configure dev vs prod differences in a single place (babel config, in this case). What do you think? |
"(node/*: any*/)", | ||
"(node as any)" | ||
)}\n}` | ||
: ""; |
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.
I think we should just mimic the upstream behaviour, which is to always do this when queries are being persisted https://github.com/facebook/relay/blob/42033ebcc298af7e8ed90632b70077b7545a282d/packages/relay-compiler/codegen/writeRelayGeneratedFile.js#L107-L133
typeText: 'export type CompleteExample = { readonly id: string }', | ||
hash: 'abcde', | ||
sourceHash: 'edcba', | ||
devOnlyAssignments: '(node/*: any*/).params.text = "query CompleteExampleQuery { id }";', |
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.
As per the upstream behaviour, the actual query text should be taken from the generated AST node: https://github.com/facebook/relay/blob/42033ebcc298af7e8ed90632b70077b7545a282d/packages/relay-compiler/codegen/writeRelayGeneratedFile.js#L110
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.
Scratch this, I wasn’t paying attention to the filename, as per my below comment.
Ah, hmm, I now see that the main difference is that upstream is doing this work in I wonder if we could move the Flow type annotation behaviour upstream to For now, though, I guess your string replace is the best we can do. |
Closes #93
Add support for https://github.com/facebook/relay/blob/f8585ab4f90f2e707d5555c9f7b423de3ea553bd/packages/relay-compiler/language/RelayLanguagePluginInterface.js#L176.
My main questions is whether this is a breaking change or not and if it should be enabled by default. Also should we make the dev check
process.env.NODE_ENV !== 'production'
configurable?