Skip to content

Commit

Permalink
Ensure incremental builds include all @rootFragment fragments
Browse files Browse the repository at this point in the history
Reviewed By: tyao1

Differential Revision: D45709148

fbshipit-source-id: 6d305dcb60fe92f9c41cffe25f7a69e304dc18a7
  • Loading branch information
captbaritone authored and facebook-github-bot committed May 9, 2023
1 parent 3b9b848 commit f0e802a
Show file tree
Hide file tree
Showing 16 changed files with 123 additions and 40 deletions.
2 changes: 0 additions & 2 deletions compiler/crates/dependency-analyzer/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ fn visit_selections(
Selection::LinkedField(linked_field) => {
if let Some(fragment_name) = get_resolver_fragment_dependency_name(
schema.field(linked_field.definition.item),
schema,
) {
update_dependency_graph(
fragment_name.into(),
Expand All @@ -209,7 +208,6 @@ fn visit_selections(
Selection::ScalarField(scalar_field) => {
if let Some(fragment_name) = get_resolver_fragment_dependency_name(
schema.field(scalar_field.definition.item),
schema,
) {
update_dependency_graph(
fragment_name.into(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
==================================== INPUT ====================================
# F1
query Q1 {
client_user {
...F1
}
}
fragment F1 on ClientUser {
pop_star_name
}

fragment ClientUserPopStarName on ClientUser {
id
}

# %definitions%

%extensions%

extend type Query {
client_user: ClientUser
}

type ClientUser @__RelayResolverModel {
id: ID!
}

# This resolver's fragment is _not_ generated even though it's on a @__RelayResolverModel type.
# Dependency tracker _should_ look for this fragment.
extend type ClientUser {
pop_star_name: String @relay_resolver(fragment_name: "ClientUserPopStarName", import_path: "PopStarNameResolver")
}
==================================== OUTPUT ===================================
Fragment: ClientUserPopStarName

Fragment: F1

Operation: Q1
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# F1
query Q1 {
client_user {
...F1
}
}
fragment F1 on ClientUser {
pop_star_name
}

fragment ClientUserPopStarName on ClientUser {
id
}

# %definitions%

%extensions%

extend type Query {
client_user: ClientUser
}

type ClientUser @__RelayResolverModel {
id: ID!
}

# This resolver's fragment is _not_ generated even though it's on a @__RelayResolverModel type.
# Dependency tracker _should_ look for this fragment.
extend type ClientUser {
pop_star_name: String @relay_resolver(fragment_name: "ClientUserPopStarName", import_path: "PopStarNameResolver")
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ type ClientUser @__RelayResolverModel {
id: ID!
}

# This resolver's fragment is generated because it's on a @__RelayResolverModel type.
# This resolver's fragment is generated because it's on a @__RelayResolverModel
# type and does not define its own fragment using @rootFragment. This is indicated with
# the `generated_fragment` directive argument.
# Dependency tracker should not look for this fragment.
extend type ClientUser {
pop_star_name: String @relay_resolver(fragment_name: "this___is___generated", import_path: "PopStarNameResolver")
pop_star_name: String @relay_resolver(fragment_name: "this___is___generated", import_path: "PopStarNameResolver", generated_fragment: true)
}
==================================== OUTPUT ===================================
Fragment: F1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ type ClientUser @__RelayResolverModel {
id: ID!
}

# This resolver's fragment is generated because it's on a @__RelayResolverModel type.
# This resolver's fragment is generated because it's on a @__RelayResolverModel
# type and does not define its own fragment using @rootFragment. This is indicated with
# the `generated_fragment` directive argument.
# Dependency tracker should not look for this fragment.
extend type ClientUser {
pop_star_name: String @relay_resolver(fragment_name: "this___is___generated", import_path: "PopStarNameResolver")
pop_star_name: String @relay_resolver(fragment_name: "this___is___generated", import_path: "PopStarNameResolver", generated_fragment: true)
}
9 changes: 8 additions & 1 deletion compiler/crates/dependency-analyzer/tests/ir_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<080b76ec4b2c6ea6cde4bc6b7c0fea2b>>
* @generated SignedSource<<1370ea763208b340fd15717d45729c87>>
*/

mod ir;
Expand Down Expand Up @@ -75,6 +75,13 @@ fn new_resolver_model_field() {
test_fixture(transform_fixture, "new-resolver-model-field.graphql", "ir/fixtures/new-resolver-model-field.expected", input, expected);
}

#[test]
fn new_resolver_model_field_with_custom_fragment() {
let input = include_str!("ir/fixtures/new-resolver-model-field-with-custom-fragment.graphql");
let expected = include_str!("ir/fixtures/new-resolver-model-field-with-custom-fragment.expected");
test_fixture(transform_fixture, "new-resolver-model-field-with-custom-fragment.graphql", "ir/fixtures/new-resolver-model-field-with-custom-fragment.expected", input, expected);
}

#[test]
fn query_then_fragment() {
let input = include_str!("ir/fixtures/query-then-fragment.graphql");
Expand Down
2 changes: 2 additions & 0 deletions compiler/crates/docblock-shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ lazy_static! {
pub static ref IMPORT_PATH_ARGUMENT_NAME: ArgumentName = ArgumentName("import_path".intern());
pub static ref INJECT_FRAGMENT_DATA_ARGUMENT_NAME: ArgumentName =
ArgumentName("inject_fragment_data".intern());
pub static ref GENERATED_FRAGMENT_ARGUMENT_NAME: ArgumentName =
ArgumentName("generated_fragment".intern());
pub static ref FIELD_NAME_FIELD: StringKey = "fieldName".intern();
pub static ref ON_TYPE_FIELD: StringKey = "onType".intern();
pub static ref ON_INTERFACE_FIELD: StringKey = "onInterface".intern();
Expand Down
7 changes: 7 additions & 0 deletions compiler/crates/graphql-syntax/src/node/constant_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ impl ConstantValue {
_ => None,
}
}

pub fn get_bool_literal(&self) -> Option<bool> {
match self {
ConstantValue::Boolean(BooleanNode { value, .. }) => Some(*value),
_ => None,
}
}
}

impl fmt::Display for ConstantValue {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ fn find_base_resolver_fragment_asts(
) -> Vec<ExecutableDefinition> {
let mut base_resolver_fragments = ExecutableDefinitionNameSet::default();
for field in schema.fields() {
if let Some(fragment_name) = get_resolver_fragment_dependency_name(field, schema) {
if let Some(fragment_name) = get_resolver_fragment_dependency_name(field) {
if base_definition_asts.contains(&fragment_name.into()) {
base_resolver_fragments.insert(fragment_name.into());
}
Expand Down
13 changes: 13 additions & 0 deletions compiler/crates/relay-docblock/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use common::ObjectName;
use common::Span;
use common::WithLocation;
use docblock_shared::FRAGMENT_KEY_ARGUMENT_NAME;
use docblock_shared::GENERATED_FRAGMENT_ARGUMENT_NAME;
use docblock_shared::HAS_OUTPUT_TYPE_ARGUMENT_NAME;
use docblock_shared::IMPORT_NAME_ARGUMENT_NAME;
use docblock_shared::IMPORT_PATH_ARGUMENT_NAME;
Expand Down Expand Up @@ -260,6 +261,7 @@ pub enum FragmentDataInjectionMode {

pub struct RootFragment {
fragment: WithLocation<FragmentDefinitionName>,
generated: bool,
// For Model resolvers, we need to pass the `id` or `__relay_model_instance` field
// from the fragment data to the resolver function
inject_fragment_data: Option<FragmentDataInjectionMode>,
Expand Down Expand Up @@ -338,6 +340,13 @@ trait ResolverIr: Sized {
root_fragment.fragment.map(|x| x.0),
));

if root_fragment.generated {
arguments.push(true_argument(
GENERATED_FRAGMENT_ARGUMENT_NAME.0,
Location::generated(),
))
}

if let Some(inject_fragment_data) = root_fragment.inject_fragment_data {
match inject_fragment_data {
FragmentDataInjectionMode::Field(field_name) => {
Expand Down Expand Up @@ -668,6 +677,7 @@ impl ResolverIr for TerseRelayResolverIr {
self.root_fragment
.map(|fragment| RootFragment {
fragment,
generated: false,
inject_fragment_data: None,
})
.or_else(|| get_root_fragment_for_object(object))
Expand Down Expand Up @@ -832,6 +842,7 @@ impl ResolverIr for RelayResolverIr {
self.root_fragment
.map(|fragment| RootFragment {
fragment,
generated: false,
inject_fragment_data: None,
})
.or_else(|| get_root_fragment_for_object(object))
Expand Down Expand Up @@ -1073,6 +1084,7 @@ impl ResolverIr for StrongObjectIr {
) -> Option<RootFragment> {
Some(RootFragment {
fragment: self.root_fragment,
generated: true,
inject_fragment_data: Some(FragmentDataInjectionMode::Field(
schema_info.config.node_interface_id_field,
)),
Expand Down Expand Up @@ -1335,6 +1347,7 @@ fn get_root_fragment_for_object(object: Option<&Object>) -> Option<RootFragment>
)
.intern(),
)),
generated: true,
inject_fragment_data: Some(FragmentDataInjectionMode::Field(
*RESOLVER_MODEL_INSTANCE_FIELD_NAME,
)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ graphql`
==================================== OUTPUT ===================================
type MyType @__RelayResolverModel {
id: ID!
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/legacy-relay-resolver-with-root-fragment-on-model.js", fragment_name: "MyType__id", inject_fragment_data: "id", import_name: "MyType") @unselectable(reason: "This field is intended only for Relay's internal use")
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/legacy-relay-resolver-with-root-fragment-on-model.js", fragment_name: "MyType__id", generated_fragment: true, inject_fragment_data: "id", import_name: "MyType") @unselectable(reason: "This field is intended only for Relay's internal use")
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ interface IFoo {
==================================== OUTPUT ===================================
type ClientUser implements IFoo @__RelayResolverModel {
id: ID!
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/relay-resolver-strong-object-with-implements.js", fragment_name: "ClientUser__id", inject_fragment_data: "id", import_name: "ClientUser") @unselectable(reason: "This field is intended only for Relay's internal use")
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/relay-resolver-strong-object-with-implements.js", fragment_name: "ClientUser__id", generated_fragment: true, inject_fragment_data: "id", import_name: "ClientUser") @unselectable(reason: "This field is intended only for Relay's internal use")
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ interface IBar {
==================================== OUTPUT ===================================
type ClientUser implements IFoo & IBar @__RelayResolverModel {
id: ID!
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/relay-resolver-strong-object-with-multiple-implements.js", fragment_name: "ClientUser__id", inject_fragment_data: "id", import_name: "ClientUser") @unselectable(reason: "This field is intended only for Relay's internal use")
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/relay-resolver-strong-object-with-multiple-implements.js", fragment_name: "ClientUser__id", generated_fragment: true, inject_fragment_data: "id", import_name: "ClientUser") @unselectable(reason: "This field is intended only for Relay's internal use")
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@
==================================== OUTPUT ===================================
type ClientUser @__RelayResolverModel {
id: ID!
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/relay-resolver-strong-object.js", fragment_name: "ClientUser__id", inject_fragment_data: "id", import_name: "ClientUser") @unselectable(reason: "This field is intended only for Relay's internal use")
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/relay-resolver-strong-object.js", fragment_name: "ClientUser__id", generated_fragment: true, inject_fragment_data: "id", import_name: "ClientUser") @unselectable(reason: "This field is intended only for Relay's internal use")
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ graphql`
==================================== OUTPUT ===================================
type MyType @__RelayResolverModel {
id: ID!
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/terse-relay-resolver-with-root-fragment-on-model.js", fragment_name: "MyType__id", inject_fragment_data: "id", import_name: "MyType") @unselectable(reason: "This field is intended only for Relay's internal use")
__relay_model_instance: Int @relay_resolver(import_path: "/path/to/test/fixture/terse-relay-resolver-with-root-fragment-on-model.js", fragment_name: "MyType__id", generated_fragment: true, inject_fragment_data: "id", import_name: "MyType") @unselectable(reason: "This field is intended only for Relay's internal use")
}


Expand Down
37 changes: 10 additions & 27 deletions compiler/crates/relay-transforms/src/relay_resolvers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ use common::Location;
use common::NamedItem;
use common::WithLocation;
use docblock_shared::FRAGMENT_KEY_ARGUMENT_NAME;
use docblock_shared::GENERATED_FRAGMENT_ARGUMENT_NAME;
use docblock_shared::HAS_OUTPUT_TYPE_ARGUMENT_NAME;
use docblock_shared::IMPORT_NAME_ARGUMENT_NAME;
use docblock_shared::IMPORT_PATH_ARGUMENT_NAME;
use docblock_shared::INJECT_FRAGMENT_DATA_ARGUMENT_NAME;
use docblock_shared::LIVE_ARGUMENT_NAME;
use docblock_shared::RELAY_RESOLVER_DIRECTIVE_NAME;
use docblock_shared::RELAY_RESOLVER_MODEL_DIRECTIVE_NAME;
use docblock_shared::RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE;
use graphql_ir::associated_data_impl;
use graphql_ir::Argument;
Expand Down Expand Up @@ -725,47 +725,30 @@ pub(crate) fn get_bool_argument_is_true(

// If the field is a resolver, return its user defined fragment name. Does not
// return generated fragment names.
pub fn get_resolver_fragment_dependency_name(
field: &Field,
schema: &SDLSchema,
) -> Option<FragmentDefinitionName> {
pub fn get_resolver_fragment_dependency_name(field: &Field) -> Option<FragmentDefinitionName> {
if !field.is_extension {
return None;
}

field
.directives
.named(*RELAY_RESOLVER_DIRECTIVE_NAME)
.filter(|resolver_directive| {
let generated = resolver_directive
.arguments
.named(*GENERATED_FRAGMENT_ARGUMENT_NAME)
.and_then(|arg| arg.value.get_bool_literal())
.unwrap_or(false);
!generated
})
.and_then(|resolver_directive| {
resolver_directive
.arguments
.named(*FRAGMENT_KEY_ARGUMENT_NAME)
})
.filter(|_| {
// Resolvers on relay model types use generated fragments, and
// therefore have no user-defined fragment dependency.
!is_field_of_relay_model(schema, field)
})
.and_then(|arg| arg.value.get_string_literal().map(FragmentDefinitionName))
}

fn is_field_of_relay_model(schema: &SDLSchema, field: &Field) -> bool {
if let Some(parent_type) = field.parent_type {
let directives = match parent_type {
schema::Type::Object(object_id) => &schema.object(object_id).directives,
schema::Type::Interface(interface_id) => &schema.interface(interface_id).directives,
schema::Type::Union(union_id) => &schema.union(union_id).directives,
_ => panic!("Expected parent to be an object, interface or union."),
};

directives
.named(*RELAY_RESOLVER_MODEL_DIRECTIVE_NAME)
.is_some()
} else {
false
}
}

fn to_camel_case(non_camelized_string: String) -> String {
let mut camelized_string = String::with_capacity(non_camelized_string.len());
let mut last_character_was_not_alphanumeric = false;
Expand Down

0 comments on commit f0e802a

Please sign in to comment.