Skip to content

Commit

Permalink
Don't treat generated fragments as dependencies
Browse files Browse the repository at this point in the history
Reviewed By: alunyov

Differential Revision: D42392021

fbshipit-source-id: 444931712bb95b6893f8636ed8bf2ec071ebe8e6
  • Loading branch information
captbaritone authored and facebook-github-bot committed Jan 6, 2023
1 parent e0d62b1 commit 39c774b
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 19 deletions.
2 changes: 1 addition & 1 deletion compiler/crates/dependency-analyzer/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub struct ReachableAst {
pub base_fragment_names: FragmentDefinitionNameSet,
}

/// Get all definitions that reachable from project defintions
/// Get all definitions that reachable from project definitions
pub fn get_reachable_ast(
project_definitions: Vec<ExecutableDefinition>,
base_definitions: Vec<ExecutableDefinition>,
Expand Down
30 changes: 16 additions & 14 deletions compiler/crates/dependency-analyzer/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use graphql_ir::*;
use intern::string_key::StringKey;
use intern::string_key::StringKeyMap;
use intern::string_key::StringKeySet;
use relay_transforms::get_resolver_fragment_name;
use relay_transforms::get_resolver_fragment_dependency_name;
use schema::SDLSchema;
use schema::Schema;

Expand All @@ -37,7 +37,7 @@ impl fmt::Debug for Node {
/// building a dependency graph where edges are either explicit fragment spreads,
/// or "implicit dependencies" such as those created by Relay Resolvers.
///
/// New implicit dependencies are detected by walking the chaged documents,
/// New implicit dependencies are detected by walking the changed documents,
/// whereas preexisting implicit dependencies must be passed in as
/// `implicit_dependencies`.
pub fn get_reachable_ir(
Expand Down Expand Up @@ -135,7 +135,7 @@ fn build_dependency_graph(
dependency_graph
}

fn update_dependecy_graph(
fn update_dependency_graph(
current_node: StringKey,
parent_name: StringKey,
dependency_graph: &mut StringKeyMap<Node>,
Expand Down Expand Up @@ -172,7 +172,7 @@ fn visit_selections(
match selection {
Selection::FragmentSpread(node) => {
let current_node = node.fragment.item.0;
update_dependecy_graph(current_node, parent_name, dependency_graph, children);
update_dependency_graph(current_node, parent_name, dependency_graph, children);
}
Selection::InlineFragment(node) => {
visit_selections(
Expand All @@ -184,10 +184,11 @@ fn visit_selections(
);
}
Selection::LinkedField(linked_field) => {
if let Some(fragment_name) =
get_resolver_fragment_name(schema.field(linked_field.definition.item))
{
update_dependecy_graph(
if let Some(fragment_name) = get_resolver_fragment_dependency_name(
schema.field(linked_field.definition.item),
schema,
) {
update_dependency_graph(
fragment_name.0,
parent_name,
dependency_graph,
Expand All @@ -203,10 +204,11 @@ fn visit_selections(
);
}
Selection::ScalarField(scalar_field) => {
if let Some(fragment_name) =
get_resolver_fragment_name(schema.field(scalar_field.definition.item))
{
update_dependecy_graph(
if let Some(fragment_name) = get_resolver_fragment_dependency_name(
schema.field(scalar_field.definition.item),
schema,
) {
update_dependency_graph(
fragment_name.0,
parent_name,
dependency_graph,
Expand All @@ -227,8 +229,8 @@ fn visit_selections(
}
}

// From `key` of changed definition, recusively traverse up the depenency tree, and add all related nodes (ancestors
// of changned definitions which are not from base definitions, and all of their desendants) into the `result`
// From `key` of changed definition, recursively traverse up the dependency tree, and add all related nodes (ancestors
// of changed definitions which are not from base definitions, and all of their descendants) into the `result`
fn add_related_nodes(
visited: &mut StringKeySet,
result: &mut StringKeyMap<ExecutableDefinition>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use graphql_ir::FragmentDefinitionNameSet;
use graphql_syntax::ExecutableDefinition;
use intern::string_key::StringKeySet;
use relay_config::ProjectConfig;
use relay_transforms::get_resolver_fragment_name;
use relay_transforms::get_resolver_fragment_dependency_name;
use schema::SDLSchema;
use schema::Schema;

Expand Down Expand Up @@ -150,7 +150,7 @@ fn find_base_resolver_fragment_asts(
) -> Vec<ExecutableDefinition> {
let mut base_resolver_fragments = StringKeySet::default();
for field in schema.fields() {
if let Some(fragment_name) = get_resolver_fragment_name(field) {
if let Some(fragment_name) = get_resolver_fragment_dependency_name(field, schema) {
if base_definition_asts.contains(&fragment_name.0) {
base_resolver_fragments.insert(fragment_name.0);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/crates/relay-transforms/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub use relay_client_component::RELAY_CLIENT_COMPONENT_MODULE_ID_ARGUMENT_NAME;
pub use relay_client_component::RELAY_CLIENT_COMPONENT_SERVER_DIRECTIVE_NAME;
pub use relay_directive::RelayDirective;
pub use relay_node_identifier::RelayLocationAgnosticBehavior;
pub use relay_resolvers::get_resolver_fragment_name;
pub use relay_resolvers::get_resolver_fragment_dependency_name;
pub use relay_resolvers::relay_resolvers;
pub use relay_resolvers::FragmentDataInjectionMode;
pub use relay_resolvers::RelayResolverMetadata;
Expand Down
29 changes: 28 additions & 1 deletion compiler/crates/relay-transforms/src/relay_resolvers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ lazy_static! {
ArgumentName("inject_fragment_data".intern());
pub static ref RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE: DirectiveName =
DirectiveName("__RelayWeakObject".intern());
static ref RESOLVER_MODEL_DIRECTIVE_NAME: DirectiveName =
DirectiveName("__RelayResolverModel".intern());
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -697,7 +699,12 @@ pub(crate) fn get_bool_argument_is_true(
}
}

pub fn get_resolver_fragment_name(field: &Field) -> Option<FragmentDefinitionName> {
// 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> {
if !field.is_extension {
return None;
}
Expand All @@ -710,9 +717,29 @@ pub fn get_resolver_fragment_name(field: &Field) -> Option<FragmentDefinitionNam
.arguments
.named(*RELAY_RESOLVER_FRAGMENT_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(*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 39c774b

Please sign in to comment.