relay
relay copied to clipboard
feat(compiler): add type declaration with relay config file types
resolve https://github.com/facebook/relay/issues/4037
I love this idea, and I've wanted it for some time. I was hopeful that there would be some way to use serde to derive this, but so far I haven't found such a took that works with our somewhat complicated setup.
I'm a little hesitant to merge this given that I don't think it's fully complete. For example I don't see feature flags or mutli-project config. Would you be interested in taking it the rest of the way and capturing the full config? If so, I think this would be a great addition.
If we do go this route, I'd love to add a few more things:
- Comments on the relevent config structs reminding people that any changes to the struct should also be added to these types.
- Documentation updates adding the types as comments, similar to what next-js does (
/** @type {import('next').NextConfig} */
Thanks for taking the time to propose this!
I'm a little hesitant to merge this given that I don't think it's fully complete. For example I don't see feature flags or mutli-project config. Would you be interested in taking it the rest of the way and capturing the full config? If so, I think this would be a great addition.
I'd love to take the rest of the config and documentate it.
But yes, it isn't fully complete because I didn't find some good documentations related to the rest of the config. I saw that the relay-config crate has other fields, but I couldn't find anything that documentate these configurations, any idea where I can find it?
What I follow was the README from the react-compiler package, but I think that is a little outdated, right?
If it's just about adding the TypeScript declarations, I think you can work of the declaration file I derived from the config last year: https://github.com/facebook/relay/blob/d873d5b8927b4a5bb6ca2c4b9a4c504ea0723b6b/packages/relay-compiler/index.d.ts I would also like to continue my effort regarding the VSCode integration for config completion in JSON files: https://github.com/facebook/relay/pull/4162 Now that I have some more experience with the compiler, maybe I have more luck than back then :D
@noghartt
I couldn't find anything that documentate these configurations, any idea where I can find it?
Sadly the best source of truth is the Rust crate. Luckily most of the fields are commented with public-facing comments. You can find it here: https://github.com/facebook/relay/blob/main/compiler/crates/relay-compiler/src/config.rs#L931
You'll need to learn a bit about how Serde (which is a Rust crate that can derived a parser for a Rust structure based on its definition) works to understand what the JS/JSON shape will look like. The main thinks to look out for:
- Each child value/object will have its own deserialization logic, usually derived from a Serde derived trait at the structs declaration (simple values like strings/paths/etc generally have intuitive inputs)
rename_all = "camelCase"converts snake_case names to camelCase#[serde(flatten)]will push all the keys of the child object up into the parent object.
Thank you so much for your interest in this. The lack of documentation for the Relay config is a huge gap, and TypeScript types will offer both in-editor support and human readable documentation that we can point people to.
cc @captbaritone @tobias-tengler
I improved the types and wrote more JSDocs based on the config.rs as mentioned before. Improve the doc too, what do you think?
Another point: We don't have authoritative docs on this config. As a baseline we should link out to this definition from our docs as a "Here's where you can go to find out what options are available in the config". As a stretch, I wonder if it would be possible to use something like https://www.npmjs.com/package/docusaurus-plugin-typedoc to generate an actual docs page (or pages) from this?
Maybe we can start with the former and then (optionally) explore the later in a separate PR.
Another point: We don't have authoritative docs on this config. As a baseline we should link out to this definition from our docs as a "Here's where you can go to find out what options are available in the config". As a stretch, I wonder if it would be possible to use something like https://www.npmjs.com/package/docusaurus-plugin-typedoc to generate an actual docs page (or pages) from this?
Maybe we can start with the former and then (optionally) explore the later in a separate PR.
Would be really cool if we could have a docs specifically for these configs, explaining which things each field are doing. I think that we can derive some docs from this config, like one about feature flags and what are the features related.
Anything else I need to fix, what do you think?
cc @captbaritone @tobias-tengler
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
I've imported this, but it will race with https://github.com/facebook/relay/pull/4585 which changes the config types. I'll try to fix it up internally before I land.
Hey. After importing this, I'm seeing a bunch more errors. How have you been validating this addition? Could you find a way to ensure it's working end to end on a given config file? Maybe include a screenshot of what errors look like in VSCode?
@noghartt Can you please add a test plan that demonstrates this working end to end? For example, a screenshot of it reporting errors on an invalid config file in VSCode? (and one of it not erroring on a valid config file)?
@noghartt Can you please add a test plan that demonstrates this working end to end? For example, a screenshot of it reporting errors on an invalid config file in VSCode? (and one of it not erroring on a valid config file)?
Of course. But in case, VSCode and LSP does not infer type checking over type infered objects that comes from JSDocs annotation, so it'll won't trigger a visual error for the final user, only if they have a relay.config.ts and let the tsserver does it for us.
But in case, I took some screenshots that shows this type working, I'll let also a test plan containing how I test it:
Test plan
- [ ] build the packages using gulp with the command
yarn gulp mainrelease - [ ] create a new
relay.config.jsfile - [ ] add type annotations targeting the exported
RelayConfigtype from./distbuild package
the boilerplate
// relay.config.js
/** @type {import('./dist/relay-compiler').RelayConfig} */
module.exports = {
// ...
}
Screenshots
starting single project
multi project config
feature flags
As an example, even the next.config.js aren't being typechecked by the LSP.
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
Well done by @captbaritone on this PR https://github.com/facebook/relay/pull/4723. We can follow this improvement and bring some type emission following the ideas of JsonSchema!