Skip to content

Commit

Permalink
Generate JSON schema for compiler config (#4723)
Browse files Browse the repository at this point in the history
Summary:
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](https://json-schema.org/) 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](https://code.visualstudio.com/api/references/contribution-points#contributes.jsonValidation)
2. Derive a docs page about the [Docusaurus JSON Schema plugin](https://jy95.github.io/docusaurus-json-schema-plugin/)

https://github.com/facebook/relay/assets/162735/7f5e807b-069c-4344-9e9c-506f3369b042

## 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`?
3. Can we get the docs plugin to work? (I think it might require Docusaurus 3)
4. 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.
5. 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:

* #4227 by tbezman
* #4162 by tobias-tengler

Pull Request resolved: #4723

Reviewed By: tyao1

Differential Revision: D58936067

Pulled By: captbaritone

fbshipit-source-id: c58488da4981eed285ee705ac65321146cfb576b
  • Loading branch information
captbaritone authored and facebook-github-bot committed Jun 25, 2024
1 parent 0e57c39 commit f544585
Show file tree
Hide file tree
Showing 28 changed files with 9,874 additions and 43 deletions.
46 changes: 46 additions & 0 deletions compiler/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions compiler/crates/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ log = { version = "0.4.17", features = ["kv_unstable", "kv_unstable_std"] }
lsp-types = "0.94.1"
md-5 = "0.10"
rayon = "1.9.0"
schemars = { version = "0.8.21", features = ["indexmap2"] }
serde = { version = "1.0.185", features = ["derive", "rc"] }
serde_json = { version = "1.0.100", features = ["float_roundtrip", "unbounded_depth"] }
typetag = "0.2.15"
5 changes: 3 additions & 2 deletions compiler/crates/common/src/feature_flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ use std::fmt::Result as FmtResult;
use indexmap::IndexSet;
use intern::string_key::StringKey;
use intern::Lookup;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;

use crate::Rollout;

#[derive(Default, Debug, Serialize, Deserialize, Clone)]
#[derive(Default, Debug, Serialize, Deserialize, Clone, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct FeatureFlags {
#[serde(default)]
Expand Down Expand Up @@ -133,7 +134,7 @@ fn default_as_true() -> bool {
true
}

#[derive(Debug, Deserialize, Clone, Serialize, Default)]
#[derive(Debug, Deserialize, Clone, Serialize, Default, JsonSchema)]
#[serde(tag = "kind", rename_all = "lowercase")]
pub enum FeatureFlag {
/// Fully disabled: developers may not use this feature
Expand Down
7 changes: 5 additions & 2 deletions compiler/crates/common/src/named_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use intern::impl_lookup;
use intern::string_key::Intern;
use intern::string_key::StringKey;
use intern::Lookup;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;

Expand Down Expand Up @@ -45,7 +46,8 @@ pub trait Named {
Ord,
PartialEq,
PartialOrd,
Serialize
Serialize,
JsonSchema
)]
pub struct DirectiveName(pub StringKey);

Expand Down Expand Up @@ -73,7 +75,8 @@ impl_lookup!(DirectiveName);
Ord,
PartialOrd,
Hash,
Serialize
Serialize,
JsonSchema
)]
pub struct ArgumentName(pub StringKey);

Expand Down
3 changes: 2 additions & 1 deletion compiler/crates/common/src/rollout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

use md5::Digest;
use md5::Md5;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;

/// A utility to enable gradual rollout of large codegen changes.
/// Can be constructed as the Default which passes or a percentage between 0 and
/// 100.
#[derive(Default, Debug, Serialize, Deserialize, Clone, Copy)]
#[derive(Default, Debug, Serialize, Deserialize, Clone, Copy, JsonSchema)]
pub struct Rollout(Option<u8>);

impl Rollout {
Expand Down
14 changes: 9 additions & 5 deletions compiler/crates/fixture-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,16 +155,20 @@ pub async fn test_fixture<T, U, V>(
};

let actual = format!("{}\n", actual.trim_end());
let expected_file_path = workspace_root()
.join(source_file_path)
.with_file_name(expected_file_name);
assert_file_contains(&actual, expected_file_path, expected)
}

pub fn assert_file_contains(actual: &str, expected_file_path: PathBuf, expected: &str) {
if actual != expected {
{
let _guard = LOCK.lock();
print_diff::print_diff(expected, &actual);
print_diff::print_diff(expected, actual);
}

if env::var_os("UPDATE_SNAPSHOTS").is_some() {
let expected_file_path = workspace_root()
.join(source_file_path)
.with_file_name(expected_file_name);
File::create(&expected_file_path)
.unwrap_or_else(|e| {
panic!(
Expand All @@ -183,7 +187,7 @@ pub async fn test_fixture<T, U, V>(
}
}

fn workspace_root() -> PathBuf {
pub fn workspace_root() -> PathBuf {
if let Ok(cargo) = std::env::var("CARGO") {
let stdout = Command::new(cargo)
.args(["locate-project", "--workspace", "--message-format=plain"])
Expand Down
1 change: 1 addition & 0 deletions compiler/crates/intern/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ hashbrown = { version = "0.14.3", features = ["raw", "serde"] }
indexmap = { version = "2.2.6", features = ["arbitrary", "rayon", "serde"] }
once_cell = "1.12"
parking_lot = { version = "0.12.1", features = ["send_guard"] }
schemars = { version = "0.8.21", features = ["indexmap2"] }
serde = { version = "1.0.185", features = ["derive", "rc"] }
serde_bytes = "0.11"
serde_derive = "1.0.185"
Expand Down
21 changes: 21 additions & 0 deletions compiler/crates/intern/src/string_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ use std::fmt::Formatter;
use std::str::FromStr;

use indexmap::IndexMap;
use schemars::gen::SchemaGenerator;
use schemars::schema::InstanceType;
use schemars::schema::Schema;
use schemars::schema::SchemaObject;
use schemars::schema::SingleOrVec;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Deserializer;
use serde::Serialize;
Expand All @@ -29,6 +35,21 @@ pub use crate::Lookup;
#[repr(transparent)]
pub struct StringKey(StringId);

impl JsonSchema for StringKey {
fn schema_name() -> std::string::String {
String::from("StringKey")
}

fn json_schema(_gen: &mut SchemaGenerator) -> Schema {
SchemaObject {
instance_type: Some(SingleOrVec::Single(Box::new(InstanceType::String))),
format: None,
..Default::default()
}
.into()
}
}

pub type StringKeyMap<V> = HashMap<StringKey, V, BuildIdHasher<u32>>;
pub type StringKeySet = HashSet<StringKey, BuildIdHasher<u32>>;
pub type StringKeyIndexMap<V> = IndexMap<StringKey, V, BuildIdHasher<u32>>;
Expand Down
7 changes: 6 additions & 1 deletion compiler/crates/relay-compiler/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# @generated by autocargo from //relay/oss/crates/relay-compiler:[relay-compiler,relay-compiler-compile_relay_artifacts_test,relay-compiler-compile_relay_artifacts_with_custom_id_test,relay-compiler-relay_compiler_integration_test]
# @generated by autocargo from //relay/oss/crates/relay-compiler:[relay-compiler,relay-compiler-compile_relay_artifacts_test,relay-compiler-compile_relay_artifacts_with_custom_id_test,relay-compiler-relay_compiler_integration_test,relay-compiler-relay_config_schema_json_test]

[package]
name = "relay-compiler"
Expand All @@ -20,6 +20,10 @@ path = "tests/compile_relay_artifacts_with_custom_id_test.rs"
name = "relay_compiler_relay_compiler_integration_test"
path = "tests/relay_compiler_integration_test.rs"

[[test]]
name = "relay_compiler_relay_config_schema_json_test"
path = "tests/relay_config_schema_json_test.rs"

[dependencies]
async-trait = "0.1.71"
bincode = "1.3.3"
Expand Down Expand Up @@ -61,6 +65,7 @@ rustc-hash = "1.1.0"
schema = { path = "../schema" }
schema-diff = { path = "../schema-diff" }
schema-validate-lib = { path = "../schema-validate" }
schemars = { version = "0.8.21", features = ["indexmap2"] }
serde = { version = "1.0.185", features = ["derive", "rc"] }
serde_bser = "0.3"
serde_json = { version = "1.0.100", features = ["float_roundtrip", "unbounded_depth"] }
Expand Down
Loading

0 comments on commit f544585

Please sign in to comment.