Skip to content

Commit 77304c2

Browse files
Alex Danofffacebook-github-bot
authored andcommitted
Add flag and validation for strict resolver flavors
Reviewed By: captbaritone Differential Revision: D48844413 fbshipit-source-id: 99810d110c01e0eb72933d21e93e3dcf82d134e0
1 parent 5c069d7 commit 77304c2

17 files changed

+354
-1
lines changed

compiler/crates/common/src/feature_flags.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ pub struct FeatureFlags {
7272
/// Fully build the schema resolvers artifact
7373
#[serde(default)]
7474
pub enable_schema_resolvers: bool,
75+
76+
/// Enforce strict flavors for relay resolvers and disallow mixing flavors
77+
#[serde(default)]
78+
pub relay_resolvers_enable_strict_resolver_flavors: FeatureFlag,
7579
}
7680

7781
#[derive(Debug, Deserialize, Clone, Serialize)]

compiler/crates/relay-compiler/src/docblocks.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ fn parse_source(
7979
enable_output_type: &project_config
8080
.feature_flags
8181
.relay_resolver_enable_output_type,
82+
enable_strict_resolver_flavors: &project_config
83+
.feature_flags
84+
.relay_resolvers_enable_strict_resolver_flavors,
8285
},
8386
)?;
8487
maybe_ir

compiler/crates/relay-compiler/tests/compile_relay_artifacts/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
119119
.content
120120
.contains("# enable_resolver_normalization_ast"),
121121
enable_schema_resolvers: false,
122+
relay_resolvers_enable_strict_resolver_flavors: FeatureFlag::Disabled,
122123
};
123124

124125
let default_project_config = ProjectConfig {

compiler/crates/relay-compiler/tests/compile_relay_artifacts_with_custom_id/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
9797
relay_resolver_enable_interface_output_type: FeatureFlag::Disabled,
9898
enable_resolver_normalization_ast: false,
9999
enable_schema_resolvers: false,
100+
relay_resolvers_enable_strict_resolver_flavors: FeatureFlag::Disabled,
100101
};
101102

102103
let default_schema_config = SchemaConfig::default();

compiler/crates/relay-docblock/src/docblock_ir.rs

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use docblock_shared::ARGUMENT_DEFINITIONS;
1919
use docblock_shared::ARGUMENT_TYPE;
2020
use docblock_shared::DEFAULT_VALUE;
2121
use docblock_shared::KEY_RESOLVER_ID_FIELD;
22+
use docblock_shared::OUTPUT_TYPE_FIELD;
2223
use docblock_shared::PROVIDER_ARG_NAME;
2324
use graphql_ir::reexport::StringKey;
2425
use graphql_ir::FragmentDefinitionName;
@@ -144,6 +145,8 @@ pub(crate) fn parse_docblock_ir(
144145
}
145146
};
146147

148+
validate_strict_resolver_flavors(parse_options, &parsed_docblock_ir)?;
149+
147150
assert_all_fields_removed(
148151
fields,
149152
resolver_field.key_location(),
@@ -724,6 +727,103 @@ fn extract_fragment_arguments(
724727
})
725728
}
726729

730+
fn get_resolver_field_path(docblock_ir: &DocblockIr) -> Option<String> {
731+
match docblock_ir {
732+
DocblockIr::RelayResolver(resolver_ir) => {
733+
let parent_type_name = match resolver_ir.on {
734+
On::Type(field) => field.value.item,
735+
On::Interface(field) => field.value.item,
736+
};
737+
738+
Some(format!("{}.{}", parent_type_name, resolver_ir.field.name))
739+
}
740+
DocblockIr::TerseRelayResolver(terse_ir) => {
741+
Some(format!("{}.{}", terse_ir.type_.item, terse_ir.field.name))
742+
}
743+
DocblockIr::StrongObjectResolver(_) => None,
744+
DocblockIr::WeakObjectType(_) => None,
745+
}
746+
}
747+
748+
// Flavorings help the compiler understand what execution strategies are viable for the resolver.
749+
// We perform validation here (if enabled) to ensure that the resolver conforms to a specific flavor.
750+
fn validate_strict_resolver_flavors(
751+
parse_options: &ParseOptions<'_>,
752+
docblock_ir: &DocblockIr,
753+
) -> DiagnosticsResult<()> {
754+
struct ResolverFlavorValidationInfo {
755+
live: Option<Location>,
756+
root_fragment: Option<Location>,
757+
output_type: Option<Location>,
758+
}
759+
let validation_info: Option<ResolverFlavorValidationInfo> = match docblock_ir {
760+
DocblockIr::RelayResolver(resolver_ir) => {
761+
let output_type_location = resolver_ir.output_type.as_ref().map(|ot| {
762+
let type_loc = ot.inner().location;
763+
let (type_start, _) = type_loc.span().as_usize();
764+
let output_type_end = type_start.saturating_sub(1); // -1 for space
765+
let output_type_start =
766+
output_type_end.saturating_sub(OUTPUT_TYPE_FIELD.lookup().len());
767+
768+
type_loc.with_span(Span::from_usize(output_type_start, output_type_end))
769+
});
770+
Some(ResolverFlavorValidationInfo {
771+
live: resolver_ir.live.map(|l| l.key_location),
772+
root_fragment: resolver_ir.root_fragment.map(|rf| rf.location),
773+
output_type: output_type_location,
774+
})
775+
}
776+
DocblockIr::TerseRelayResolver(terse_ir) => Some(ResolverFlavorValidationInfo {
777+
live: terse_ir.live.map(|l| l.key_location),
778+
root_fragment: terse_ir.root_fragment.map(|rf| rf.location),
779+
output_type: None,
780+
}),
781+
DocblockIr::StrongObjectResolver(_) => None,
782+
DocblockIr::WeakObjectType(_) => None,
783+
};
784+
785+
let validation_info = if let Some(validation_info) = validation_info {
786+
validation_info
787+
} else {
788+
return Ok(());
789+
};
790+
791+
// TODO(T161157239): also check if this is a model resolver, which is not compatible with root fragment
792+
793+
if validation_info.root_fragment.is_some()
794+
&& (validation_info.live.is_some() || validation_info.output_type.is_some())
795+
{
796+
let resolver_field_path = get_resolver_field_path(docblock_ir)
797+
.expect("Should have a resolver path for RelayResolver or TerseRelayResolver");
798+
799+
if !parse_options
800+
.enable_strict_resolver_flavors
801+
.is_enabled_for(resolver_field_path.intern())
802+
{
803+
return Ok(());
804+
}
805+
806+
let mut errs = vec![];
807+
if let Some(live_loc) = validation_info.live {
808+
errs.push(Diagnostic::error(
809+
"@live is incompatible with @rootFragment",
810+
live_loc,
811+
));
812+
}
813+
814+
if let Some(output_type_loc) = validation_info.output_type {
815+
errs.push(Diagnostic::error(
816+
"@outputType is incompatible with @rootFragment",
817+
output_type_loc,
818+
));
819+
}
820+
821+
Err(errs)
822+
} else {
823+
Ok(())
824+
}
825+
}
826+
727827
fn validate_field_arguments(
728828
arguments: &Option<List<InputValueDefinition>>,
729829
source_location: SourceLocationKey,

compiler/crates/relay-docblock/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use untyped_representation::parse_untyped_docblock_representation;
3939

4040
pub struct ParseOptions<'a> {
4141
pub enable_output_type: &'a FeatureFlag,
42+
pub enable_strict_resolver_flavors: &'a FeatureFlag,
4243
}
4344

4445
pub fn parse_docblock_ast(
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
==================================== INPUT ====================================
2+
/**
3+
* Copyright (c) Meta Platforms, Inc. and affiliates.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
// expected-to-throw
10+
// relay:enable_strict_resolver_flavors
11+
12+
/**
13+
* @RelayResolver User.favorite_page: Page
14+
* @rootFragment myRootFragment
15+
* @live
16+
*
17+
* The user's favorite page! They probably clicked something in the UI
18+
* to tell us that it was their favorite page and then we put that in a
19+
* database or something. Then we got that info out again and put it out
20+
* again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
21+
*/
22+
graphql`
23+
fragment myRootFragment on User {
24+
name
25+
}
26+
`
27+
==================================== ERROR ====================================
28+
✖︎ @live is incompatible with @rootFragment
29+
30+
/path/to/test/fixture/strict-flavors-live-resolver-with-root-fragment.invalid.js:14:5
31+
13 │ * @rootFragment myRootFragment
32+
14 │ * @live
33+
│ ^^^^
34+
15 │ *
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
// expected-to-throw
9+
// relay:enable_strict_resolver_flavors
10+
11+
/**
12+
* @RelayResolver User.favorite_page: Page
13+
* @rootFragment myRootFragment
14+
* @live
15+
*
16+
* The user's favorite page! They probably clicked something in the UI
17+
* to tell us that it was their favorite page and then we put that in a
18+
* database or something. Then we got that info out again and put it out
19+
* again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
20+
*/
21+
graphql`
22+
fragment myRootFragment on User {
23+
name
24+
}
25+
`
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
==================================== INPUT ====================================
2+
/**
3+
* Copyright (c) Meta Platforms, Inc. and affiliates.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
// expected-to-throw
10+
// relay:enable_strict_resolver_flavors
11+
// relay:enable_output_type
12+
13+
/**
14+
* @RelayResolver
15+
* @fieldName favorite_page
16+
* @onType User
17+
* @rootFragment myRootFragment
18+
* @outputType SomeType
19+
* @live
20+
*
21+
* The user's favorite page! They probably clicked something in the UI
22+
* to tell us that it was their favorite page and then we put that in a
23+
* database or something. Then we got that info out again and put it out
24+
* again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
25+
*/
26+
graphql`
27+
fragment myRootFragment on User {
28+
name
29+
}
30+
`
31+
==================================== ERROR ====================================
32+
✖︎ @live is incompatible with @rootFragment
33+
34+
/path/to/test/fixture/strict-flavors-multiple-errors.invalid.js:18:5
35+
17 │ * @outputType SomeType
36+
18 │ * @live
37+
│ ^^^^
38+
19 │ *
39+
40+
41+
✖︎ @outputType is incompatible with @rootFragment
42+
43+
/path/to/test/fixture/strict-flavors-multiple-errors.invalid.js:17:5
44+
16 │ * @rootFragment myRootFragment
45+
17 │ * @outputType SomeType
46+
│ ^^^^^^^^^^
47+
18 │ * @live
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
// expected-to-throw
9+
// relay:enable_strict_resolver_flavors
10+
// relay:enable_output_type
11+
12+
/**
13+
* @RelayResolver
14+
* @fieldName favorite_page
15+
* @onType User
16+
* @rootFragment myRootFragment
17+
* @outputType SomeType
18+
* @live
19+
*
20+
* The user's favorite page! They probably clicked something in the UI
21+
* to tell us that it was their favorite page and then we put that in a
22+
* database or something. Then we got that info out again and put it out
23+
* again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
24+
*/
25+
graphql`
26+
fragment myRootFragment on User {
27+
name
28+
}
29+
`

0 commit comments

Comments
 (0)