Relay: [RFC] Allow for more complex configuration of relay-compiler / babel plugin

Created on 21 Aug 2018  Â·  15Comments  Â·  Source: facebook/relay

While I’m not a fan of lots of configuration –and thus think that where possible we should have generally applicable defaults, such as with persisted queries– I’m starting to feel like relay-compiler + the babel plugin could do with a more advanced way of configuration, in the form of an evaluated JS config module:

  • Being able to specify custom scalar mappings (#2162)
  • Specifying custom persisted query logic (
    https://github.com/facebook/relay/pull/2354#issuecomment-36927167, 0https://github.com/facebook/relay/pull/2354#issuecomment-395483449, and #1846)
  • Currently the path to the single artefact directory needs to be passed to _both_ the CLI and the babel plugin. It would be great if this could be specified in one place instead. (https://github.com/relay-tools/relay-compiler-language-typescript#configuration)
export default {
  schema: "./data/schema.json",
  language: "typescript",
  artifactDirectory: "./src/__relay_artifacts__",
  customScalars: { URL: "String" },
  persistedQueries: (…) => { … },
}
enhancement help wanted

Most helpful comment

Initially I was skeptical of this - configuration can quickly get out of hand especially if configuration values can come from too many different places (environment variables, command line args, etc). I think we can follow metro's approach (the cosmiconfig library looks good), so long as we ensure a good user experience:

  • Warn if there are multiple potential sources of configuration in a single project (e.g. a key in package.json and a standalone foo.config.js file). Maybe cosmiconfig already does this, if not we should add that.
  • Be consistent about naming of configuration keys and the corresponding command-line overrides, and allow command-line arg versions for all arguments.
  • Add a flag that shows where configuration values are coming from.

Re implementation, it would be great to have this as a standalone package. Before sending a PR, I think the first step is to write up a quick sketch of what the API would be and an overview of the implementation, list out some test cases. We can review that before proceeding with coding.

All 15 comments

cc @jstejada @kassens

cc @alunyov

consuming a user babel config in js is pretty easy

you can just import like a normal .js file

@alloy Is the "language" option already supported? If not how would that work? Should it compile on the fly?

@bencallaway It is. See https://github.com/relay-tools/relay-compiler-language-typescript#relay-compiler-language-typescript

@alloy I have been using the relay-compiler-language-typesecript plugin, and very thankful for your work there btw. I was curious about the babel-plugin-relay config itself. As of 1.7.0-rc.1 it appears "language" is not a plugin option (yet? <- Partly my previous question). However, after looking back at the code I now see there's a buildCommand option that I had failed to notice before (https://github.com/facebook/relay/blob/v1.7.0-rc.1/packages/babel-plugin-relay/BabelPluginRelay.js). It would seem I can pass in my own relay-compiler --language typescript and the compilation will occur on the fly. (I am currently doing this with a separate --watch process while developing.

In any case I'm for extending the config as you're proposing here

Ok, retracting some of that last comment. It appears that buildCommand does not handle compilation on the fly, it's just used for error reporting. I'll need to spend some more time looking through the plugin's source.

Oh I see. No currently it remains a two-step process. relay-compiler should be used to generate the artefact files, the babel plugin to switch out the graphql tags with the corresponding artefacts.

Initially I was skeptical of this - configuration can quickly get out of hand especially if configuration values can come from too many different places (environment variables, command line args, etc). I think we can follow metro's approach (the cosmiconfig library looks good), so long as we ensure a good user experience:

  • Warn if there are multiple potential sources of configuration in a single project (e.g. a key in package.json and a standalone foo.config.js file). Maybe cosmiconfig already does this, if not we should add that.
  • Be consistent about naming of configuration keys and the corresponding command-line overrides, and allow command-line arg versions for all arguments.
  • Add a flag that shows where configuration values are coming from.

Re implementation, it would be great to have this as a standalone package. Before sending a PR, I think the first step is to write up a quick sketch of what the API would be and an overview of the implementation, list out some test cases. We can review that before proceeding with coding.

Re implementation, it would be great to have this as a standalone package.

Do you mean in the same way how relay-compiler and react-relay are separate packages but part of this monorepo?

Yeah, in this repo as a sibling package to relay-compiler/

…allow command-line arg versions for all arguments.

I'm going to start with PRs to make all features available through the CLI first, as that needs to get done anyways.

Regarding config coming from different places, I know that jest use a jest-config package that centralise this logic. You can find it here. It was explained by @cpojer in a video where they explained the overall architecture of jest.

Be consistent about naming of configuration keys and the corresponding command-line overrides

@josephsavona I noticed that we have camelCase _and_ kebab-case options currently. camelCase would make consistency between CLI and JS configuration easier and probably more natural, but kebab-case is ofc more natural to CLIs. Should we choose one case and, if so, which one?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sibelius picture sibelius  Â·  3Comments

leebyron picture leebyron  Â·  3Comments

rayronvictor picture rayronvictor  Â·  3Comments

jstejada picture jstejada  Â·  3Comments

janicduplessis picture janicduplessis  Â·  3Comments