relay-compiler-language-typescript icon indicating copy to clipboard operation
relay-compiler-language-typescript copied to clipboard

Add support for devOnlyAssignments

Open janicduplessis opened this issue 6 years ago • 2 comments

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?

janicduplessis avatar Feb 28 '19 01:02 janicduplessis

Oh! I didn’t even know of this. Great stuff, I guess we should also update the documentation upstream on the interface then.

~~Ok, so what’s interesting is that upstream appears to always set it when queries are being persisted, so perhaps they strip it out on builds?~~ [Scratch that, as stated below I was confused about what file I was looking at, it actually looks like the OSS version of formatGeneratedModule does nothing with the passed in devOnlyAssignments, so I guess FB has an internal fork?] That would actually be more suitable to our [Artsy’s] setup, as we check our artefacts in and having different artefacts for dev vs prod would lead to a dirty SCM checkout.

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?

alloy avatar Mar 12 '19 10:03 alloy

Ah, hmm, I now see that the main difference is that upstream is doing this work in writeRelayGeneratedFile, instead of formatGeneratedModule 🤔

I wonder if we could move the Flow type annotation behaviour upstream to formatGeneratedModule instead, because writeRelayGeneratedFile shouldn’t really know about Flow specifically. /cc @jstejada

For now, though, I guess your string replace is the best we can do.

alloy avatar Mar 12 '19 10:03 alloy