Skip to content
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

Generate JSON schema for compiler config #4723

Closed
wants to merge 2 commits into from
Closed

Conversation

captbaritone
Copy link
Contributor

@captbaritone captbaritone commented Jun 23, 2024

One huge usability issue with Relay today is that the compiler config is wholely undocumented. To understand what options are available you either need to try an option and try to decipher the sometime cryptic validation errors, or read the Rust code.

This PR attempts to start to close that gap. Our Rust structs and comments are a good source of truth for this info, so ideally we still use that as our documentation/validation. So, here we derive Json Schema from our Rust structs/enums/types/serde tags.

In VSCode this allows for nice autocomplete, hover tool tips, and yellow squiggles if you make an error (much nicer than a cryptic validation error when you run the compiler).

Some next steps this could enable:

  1. Enable this editor experience directly if you have the Relay VSCode extension installed via the JSON Schema contribution point
  2. Derive a docs page about the Docusaurus JSON Schema plugin
Screen.Recording.2024-06-23.at.11.11.20.AM.mov

Next Steps

There are a few unanswered things here that we'll need to consider:

  1. How do we publish this schema such that users can find it and ensure that they use the correct schema for the version of the compiler they are using?
    1. Especially tricky if we want to enable it automatically in VSCode
    2. Maybe we could have the compiler validate that you've added the right value for $schema?
  2. Can we get the docs plugin to work? (I think it might require Docusaurus 3)
  3. Are there bugs? There are some serde attributes and little deserialization tricks which we might not have modeled correctly here. Difficult to know if we've captured them all.
  4. Add more descriptions to our structs. Some fields are undocumented. This would provide us more incentive to get those struct/field/variant /// comments very helpful and up to date.

Acknowledgment

This PR aims to be a more maintainable and reliable alternative to #4569. Thanks for @noghartt for pushing the discussion of how we might fill this gap.

This approach was also previously explored in other PRs:

@tobias-tengler
Copy link
Contributor

Love the resumed work on this! I updated my old attempt to your version in #4724
I think it would be easiest and best for adoption to publish the JSON schema as part of the relay-compiler npm package. The VS Code extension and other tooling could pick it up from there.

@captbaritone
Copy link
Contributor Author

I updated my old attempt to your version in #4724

Oh this is great! I wasn't sure if there was a way for the VSCode extension to locate the Json Schema file in node_modules. Looking forward to looking more closely at that PR.

Also, my sincere apologies for not acknowledging previous attempts at this. I had forgotten about your PR and also @tbezman! I've updated the description to reference both.

Add json schema for savedStateConfig

Don't remove deprecated config options. Do that in separate PR
@@ -360,6 +360,13 @@ const relayCompiler = gulp.parallel(
})
.pipe(gulp.dest(path.join(DIST, 'relay-compiler')));
},
function copyCompilerConfigSchema() {
return gulp
.src(['relay-compiler-config-schema.json'], {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobias-tengler This should put the config schema in the root of the relay-compiler npm module.

Screenshot 2024-06-23 at 8 41 47 PM

let expected_file_path = workspace_root()
.join(source_file_path)
.with_file_name("../../../relay-compiler-config-schema.json");
assert_file_contains(&actual, expected_file_path, expected)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reuses some of the logic from our fixture files:

  1. It will fail and show a diff if the current file does not match
  2. Setting UPDATE_SNAPSHOTS=1 will cause it to update the file

@captbaritone captbaritone changed the title [WIP] Generate JSON schema for compiler config Generate JSON schema for compiler config Jun 24, 2024
@facebook-github-bot
Copy link
Contributor

@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@orta
Copy link
Contributor

orta commented Jun 24, 2024

How do we publish this schema such that users can find it and ensure that they use the correct schema for the version of the compiler they are using?

Is pretty easy to solve, send a PR to https://github.com/SchemaStore/schemastore/blob/master/src/api/json/catalog.json with a link to a version you host on the relay website

@captbaritone
Copy link
Contributor Author

@orta Neat! How does version resolution work? If we add multiple versions there, who choses which one to use?

@facebook-github-bot
Copy link
Contributor

@captbaritone merged this pull request in f544585.

@orta
Copy link
Contributor

orta commented Jun 26, 2024

It basically doesn't - I think?

Might be able to do it with more control if you used the vscode extension to set the schema instead probably

@captbaritone
Copy link
Contributor Author

Yeah, that's what I think we'll do. Tobias already has a PR open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants