js_module_import_path -> js_module_import_identifier

voideanvalue · facebook-github-bot · commit 05689b07f341 · 2023-11-27T12:18:46.000-08:00
Reviewed By: alunyov

Differential Revision: D51589285

fbshipit-source-id: 9811e86c262b9373ebb38a088f280bb4e87618e7
diff --git a/compiler/crates/graphql-ir/src/ir.rs b/compiler/crates/graphql-ir/src/ir.rs
@@ -202,6 +202,18 @@ impl From<ExecutableDefinitionName> for StringKey {
     }
 }
 
+impl From<OperationDefinitionName> for StringKey {
+    fn from(operation_definition_name: OperationDefinitionName) -> Self {
+        operation_definition_name.0
+    }
+}
+
+impl From<FragmentDefinitionName> for StringKey {
+    fn from(fragment_definition_name: FragmentDefinitionName) -> Self {
+        fragment_definition_name.0
+    }
+}
+
 impl Lookup for ExecutableDefinitionName {
     fn lookup(self) -> &'static str {
         match self {
diff --git a/compiler/crates/relay-codegen/src/build_ast.rs b/compiler/crates/relay-codegen/src/build_ast.rs
@@ -5,6 +5,8 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+use std::path::PathBuf;
+
 use common::NamedItem;
 use common::WithLocation;
 use graphql_ir::Argument;
@@ -720,15 +722,15 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
         let is_output_type = resolver_metadata
             .output_type_info
             .normalization_ast_should_have_is_output_type_true();
-        let module = resolver_metadata.import_path;
-
-        let import_path = self
-            .project_config
-            .js_module_import_path(self.definition_source_location, module);
 
         let variable_name = resolver_metadata.generate_local_resolver_name(self.schema);
         let resolver_js_module = JSModuleDependency {
-            path: import_path,
+            path: self.project_config.js_module_import_identifier(
+                &self
+                    .project_config
+                    .artifact_path_for_definition(self.definition_source_location),
+                &PathBuf::from(resolver_metadata.import_path.lookup()),
+            ),
             import_name: match resolver_metadata.import_name {
                 Some(name) => ModuleImportName::Named {
                     name,
@@ -1058,14 +1060,14 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
         {
             let fragment_source_location_key = no_inline_metadata.location;
 
-            let path_for_artifact = self.project_config.create_path_for_artifact(
-                fragment_source_location_key,
-                frag_spread.fragment.item.0.lookup().to_string(),
-            );
-
-            let normalization_import_path = self.project_config.js_module_import_path(
-                self.definition_source_location,
-                path_for_artifact.to_str().unwrap().intern(),
+            let normalization_import_path = self.project_config.js_module_import_identifier(
+                &self
+                    .project_config
+                    .artifact_path_for_definition(self.definition_source_location),
+                &self.project_config.create_path_for_artifact(
+                    fragment_source_location_key,
+                    frag_spread.fragment.item.0.lookup().to_string(),
+                ),
             );
 
             return self
@@ -1128,7 +1130,6 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
         relay_resolver_metadata: &RelayResolverMetadata,
         fragment_primitive: Option<Primitive>,
     ) -> Primitive {
-        let module = relay_resolver_metadata.import_path;
         let field = relay_resolver_metadata.field(self.schema);
         let field_arguments = &relay_resolver_metadata.field_arguments;
         let field_alias = relay_resolver_metadata.field_alias;
@@ -1141,9 +1142,12 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
             CODEGEN_CONSTANTS.relay_resolver
         };
 
-        let import_path = self
-            .project_config
-            .js_module_import_path(self.definition_source_location, module);
+        let import_path = self.project_config.js_module_import_identifier(
+            &self
+                .project_config
+                .artifact_path_for_definition(self.definition_source_location),
+            &PathBuf::from(relay_resolver_metadata.import_path.lookup()),
+        );
 
         let args = if field.arguments.is_empty() {
             Primitive::SkippableNull
@@ -1175,14 +1179,14 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
         let resolver_module = if let Some((fragment_name, injection_mode)) =
             relay_resolver_metadata.fragment_data_injection_mode
         {
-            let path_for_artifact = self.project_config.create_path_for_artifact(
-                fragment_name.location.source_location(),
-                fragment_name.item.to_string(),
-            );
-
-            let fragment_import_path = self.project_config.js_module_import_path(
-                self.definition_source_location,
-                path_for_artifact.to_str().unwrap().intern(),
+            let fragment_import_path = self.project_config.js_module_import_identifier(
+                &self
+                    .project_config
+                    .artifact_path_for_definition(self.definition_source_location),
+                &self.project_config.create_path_for_artifact(
+                    fragment_name.location.source_location(),
+                    fragment_name.item.to_string(),
+                ),
             );
 
             Primitive::RelayResolverModel {
@@ -1244,14 +1248,14 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
                 .location
                 .source_location();
 
-            let path_for_artifact = self.project_config.create_path_for_artifact(
-                normalization_artifact_source_location,
-                normalization_info.normalization_operation.item.to_string(),
-            );
-
-            let normalization_import_path = self.project_config.js_module_import_path(
-                self.definition_source_location,
-                path_for_artifact.to_str().unwrap().intern(),
+            let normalization_import_path = self.project_config.js_module_import_identifier(
+                &self
+                    .project_config
+                    .artifact_path_for_definition(self.definition_source_location),
+                &self.project_config.create_path_for_artifact(
+                    normalization_artifact_source_location,
+                    normalization_info.normalization_operation.item.to_string(),
+                ),
             );
             let concrete_type = if normalization_info.inner_type.is_abstract_type() {
                 Primitive::Null
@@ -2011,9 +2015,11 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
                         provider.module_name
                     } else {
                         // This will build a path from the operation artifact to the provider module
-                        self.project_config.js_module_import_path(
-                            operation.name.map(|name| name.0),
-                            provider.module_path().to_str().unwrap().intern(),
+                        self.project_config.js_module_import_identifier(
+                            &self
+                                .project_config
+                                .artifact_path_for_definition(operation.name),
+                            &provider.module_path(),
                         )
                     };
 
diff --git a/compiler/crates/relay-compiler/src/build_project/generate_artifacts.rs b/compiler/crates/relay-compiler/src/build_project/generate_artifacts.rs
@@ -83,7 +83,7 @@ pub fn generate_artifacts(
                     return Artifact {
                         artifact_source_keys,
                         path: project_config
-                            .path_for_artifact(source_file, normalization.name.item.0),
+                            .artifact_path_for_definition(normalization.name),
                         content: ArtifactContent::SplitOperation {
                             normalization_operation: Arc::clone(normalization),
                             typegen_operation,
@@ -96,10 +96,6 @@ pub fn generate_artifacts(
                     RefetchableDerivedFromMetadata::find(&normalization.directives)
                 {
                     let source_name = derived_from_metadata.0;
-                    let source_fragment = programs
-                        .source
-                        .fragment(source_name)
-                        .expect("Expected the source document for the SplitOperation to exist.");
                     let source_hash = source_hashes.get(&source_name.into()).cloned().unwrap();
 
                     return generate_normalization_artifact(
@@ -108,24 +104,18 @@ pub fn generate_artifacts(
                         project_config,
                         &operations,
                         source_hash,
-                        source_fragment.name.location.source_location(),
                     )
                 } else if let Some(client_edges_directive) =
                     ClientEdgeGeneratedQueryMetadataDirective::find(&normalization.directives)
                 {
                     let source_name = client_edges_directive.source_name.item;
-                    let source_file = client_edges_directive
-                        .source_name
-                        .location
-                        .source_location();
                     let source_hash = source_hashes.get(&source_name).cloned().unwrap();
                     return generate_normalization_artifact(
                         &mut operation_printer,
                         ArtifactSourceKey::ExecutableDefinition(source_name),
                         project_config,
                         &operations,
                         source_hash,
-                        source_file,
                     )
                 } else {
                     let source_hash = source_hashes
@@ -138,7 +128,6 @@ pub fn generate_artifacts(
                         project_config,
                         &operations,
                         source_hash,
-                        normalization.name.location.source_location(),
                     )
                 }
             } else if let Some(reader) = operations.reader {
@@ -158,7 +147,6 @@ pub fn generate_artifacts(
                         project_config,
                         &operations,
                         source_hash,
-                        reader.name.location.source_location(),
                     )
                 }
             }
@@ -208,7 +196,6 @@ fn generate_normalization_artifact(
     project_config: &ProjectConfig,
     operations: &OperationGroup<'_>,
     source_hash: String,
-    source_file: SourceLocationKey,
 ) -> Artifact {
     let text = operations
         .operation_text
@@ -220,7 +207,7 @@ fn generate_normalization_artifact(
 
     Artifact {
         artifact_source_keys: vec![artifact_source],
-        path: project_config.path_for_artifact(source_file, normalization.name.item.0),
+        path: project_config.artifact_path_for_definition(normalization.name),
         content: ArtifactContent::Operation {
             normalization_operation: Arc::clone(normalization),
             reader_operation: operations.expect_reader(),
@@ -238,15 +225,14 @@ fn generate_updatable_query_artifact(
     project_config: &ProjectConfig,
     operations: &OperationGroup<'_>,
     source_hash: String,
-    source_file: SourceLocationKey,
 ) -> Artifact {
     let reader = operations
         .reader
         .expect("Updatable operations must have a reader entry.");
 
     Artifact {
         artifact_source_keys: vec![artifact_source],
-        path: project_config.path_for_artifact(source_file, reader.name.item.0),
+        path: project_config.artifact_path_for_definition(reader.name),
         content: ArtifactContent::UpdatableQuery {
             reader_operation: operations.expect_reader(),
             typegen_operation: operations.expect_typegen(),
@@ -270,8 +256,7 @@ fn generate_reader_artifact(
         .expect("a type fragment should be generated for this fragment");
     Artifact {
         artifact_source_keys,
-        path: project_config
-            .path_for_artifact(reader_fragment.name.location.source_location(), name.0),
+        path: project_config.artifact_path_for_definition(reader_fragment.name),
         content: ArtifactContent::Fragment {
             reader_fragment: Arc::clone(reader_fragment),
             typegen_fragment: Arc::clone(typegen_fragment),
diff --git a/compiler/crates/relay-config/src/project_config.rs b/compiler/crates/relay-config/src/project_config.rs
@@ -9,7 +9,6 @@ use std::fmt;
 use std::path::Path;
 use std::path::PathBuf;
 use std::path::MAIN_SEPARATOR;
-use std::str::FromStr;
 use std::sync::Arc;
 use std::usize;
 
@@ -23,7 +22,6 @@ use fnv::FnvBuildHasher;
 use indexmap::IndexMap;
 use intern::string_key::Intern;
 use intern::string_key::StringKey;
-use intern::Lookup;
 use regex::Regex;
 use serde::de::Error;
 use serde::Deserialize;
@@ -365,68 +363,59 @@ impl ProjectConfig {
         }
     }
 
-    pub fn path_for_artifact(
+    pub fn artifact_path_for_definition(
         &self,
-        source_file: SourceLocationKey,
-        definition_name: StringKey,
+        definition_name: WithLocation<impl Into<StringKey>>,
     ) -> PathBuf {
+        let source_location = definition_name.location.source_location();
+        let artifact_name = definition_name.item.into();
         let filename = if let Some(filename_for_artifact) = &self.filename_for_artifact {
-            filename_for_artifact(source_file, definition_name)
+            filename_for_artifact(source_location, artifact_name)
         } else {
             match &self.typegen_config.language {
                 TypegenLanguage::Flow | TypegenLanguage::JavaScript => {
-                    format!("{}.graphql.js", definition_name)
+                    format!("{}.graphql.js", artifact_name)
                 }
-                TypegenLanguage::TypeScript => format!("{}.graphql.ts", definition_name),
+                TypegenLanguage::TypeScript => format!("{}.graphql.ts", artifact_name),
             }
         };
-        self.create_path_for_artifact(source_file, filename)
+        self.create_path_for_artifact(source_location, filename)
     }
 
-    /// Generates a relative import path in Common JS projects, and a module name in Haste projects.
-    pub fn js_module_import_path(
+    /// Generates identifier for importing module at `target_module_path` from module at `importing_artifact_path`.
+    /// Import Identifier is a relative path in CommonJS projects and a module name in Haste projects.
+    pub fn js_module_import_identifier(
         &self,
-        definition_source_location: WithLocation<StringKey>,
-        target_module: StringKey,
+        importing_artifact_path: &PathBuf,
+        target_module_path: &PathBuf,
     ) -> StringKey {
         match self.js_module_format {
             JsModuleFormat::CommonJS => {
-                let definition_artifact_location = self.path_for_artifact(
-                    definition_source_location.location.source_location(),
-                    definition_source_location.item,
-                );
-
-                let module_location =
-                    PathBuf::from_str(target_module.lookup()).unwrap_or_else(|_| {
-                        panic!(
-                            "expected to be able to build a path from target_module : {}",
-                            target_module.lookup()
-                        );
-                    });
-
-                let module_path = module_location.parent().unwrap_or_else(||{
+                let importing_artifact_directory = importing_artifact_path.parent().unwrap_or_else(||{
                     panic!(
-                        "expected module_location: {:?} to have a parent path, maybe it's not a file?",
-                        module_location
+                        "expected importing_artifact_path: {:?} to have a parent path, maybe it's not a file?",
+                        importing_artifact_path
                     );
                 });
-
-                let definition_artifact_location_path = definition_artifact_location.parent().unwrap_or_else(||{panic!("expected definition_artifact_location: {:?} to have a parent path, maybe it's not a file?", definition_artifact_location);
-            });
-
-                let resolver_module_location =
-                    pathdiff::diff_paths(module_path, definition_artifact_location_path).unwrap();
-
-                let module_file_name = module_location.file_name().unwrap_or_else(|| {
+                let target_module_directory = target_module_path.parent().unwrap_or_else(||{
+                    panic!(
+                        "expected target_module_path: {:?} to have a parent path, maybe it's not a file?",
+                        target_module_path
+                    );
+                });
+                let target_module_file_name = target_module_path.file_name().unwrap_or_else(|| {
                     panic!(
-                        "expected module_location: {:?} to have a file name",
-                        module_location
+                        "expected target_module_path: {:?} to have a file name",
+                        target_module_path
                     )
                 });
+                let relative_path =
+                    pathdiff::diff_paths(target_module_directory, importing_artifact_directory)
+                        .unwrap();
 
-                format_normalized_path(&resolver_module_location.join(module_file_name)).intern()
+                format_normalized_path(&relative_path.join(target_module_file_name)).intern()
             }
-            JsModuleFormat::Haste => Path::new(&target_module.to_string())
+            JsModuleFormat::Haste => target_module_path
                 .file_stem()
                 .unwrap()
                 .to_string_lossy()
diff --git a/compiler/crates/relay-transforms/src/match_/split_module_import.rs b/compiler/crates/relay-transforms/src/match_/split_module_import.rs
@@ -142,7 +142,7 @@ impl Transformer for SplitModuleImportTransform<'_, '_> {
                         },
                         OperationDefinition {
                             name: WithLocation::new(
-                                module_metadata.location,
+                                module_metadata.fragment_source_location,
                                 OperationDefinitionName(normalization_name),
                             ),
                             type_: parent_type,
diff --git a/compiler/crates/relay-typegen/src/visit.rs b/compiler/crates/relay-typegen/src/visit.rs
diff --git a/compiler/crates/relay-typegen/src/write.rs b/compiler/crates/relay-typegen/src/write.rs
