Fix loading config files via CLI argument

Summary:
Fix for #3801

 - limit loader to only search for config in `relay.config.js` and `relay.config.json` (+ package.json `relay` section)
- update error message for the missing config file.
- add support for JS files when passing config file to the compiler

Reviewed By: captbaritone

Differential Revision: D35090499

fbshipit-source-id: 07ebdebdcbedff9372d70d71654c41f7516c4725

compiler/crates/js-config-loader/src/lib.rs

@@ -11,33 +11,61 @@ mod loader;
 pub use error::{ConfigError, ErrorCode};
 use loader::{JsLoader, JsonLoader, Loader, PackageJsonLoader, YamlLoader};
 use serde::Deserialize;
-use std::path::{Path, PathBuf};
+use std::{
+    fmt::Display,
+    path::{Path, PathBuf},
+};
 
 #[derive(Debug)]
 pub struct Config<T> {
     pub path: PathBuf,
     pub value: T,
 }
 
-pub fn search<T>(name: &str, dir: &Path) -> Result<Option<Config<T>>, ConfigError>
+pub enum LoaderSource {
+    PackageJson(String),
+    Json(String),
+    Js(String),
+    Yaml(String),
+}
+
+impl Display for LoaderSource {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "{}",
+            match self {
+                Self::PackageJson(name) => format!("`package.json` (\"{}\" key)", name),
+                Self::Js(name) | Self::Json(name) | Self::Yaml(name) => format!("`{}`", name),
+            }
+        )
+    }
+}
+
+pub fn load<T>(dir: &Path, sources: &[LoaderSource]) -> Result<Option<Config<T>>, ConfigError>
 where
     T: for<'de> Deserialize<'de> + 'static,
 {
-    let loaders: Vec<(String, Box<dyn Loader<T>>)> = vec![
-        (
-            String::from("package.json"),
-            Box::new(PackageJsonLoader { key: name }),
-        ),
-        (format!(".{}rc", name), Box::new(JsonLoader)),
-        (format!(".{}rc.json", name), Box::new(JsonLoader)),
-        (format!(".{}rc.yaml", name), Box::new(YamlLoader)),
-        (format!(".{}rc.yml", name), Box::new(YamlLoader)),
-        (format!(".{}rc.js", name), Box::new(JsLoader)),
-        (format!(".{}rc.cjs", name), Box::new(JsLoader)),
-        (format!("{}.config.js", name), Box::new(JsLoader)),
-        (format!("{}.config.json", name), Box::new(JsonLoader)),
-        (format!("{}.config.cjs", name), Box::new(JsLoader)),
-    ];
+    let mut loaders: Vec<(String, Box<dyn Loader<T>>)> = Vec::with_capacity(sources.len());
+    for source in sources {
+        match source {
+            LoaderSource::PackageJson(name) => {
+                loaders.push((
+                    String::from("package.json"),
+                    Box::new(PackageJsonLoader { key: name }),
+                ));
+            }
+            LoaderSource::Js(name) => {
+                loaders.push((name.clone(), Box::new(JsLoader)));
+            }
+            LoaderSource::Json(name) => {
+                loaders.push((name.clone(), Box::new(JsonLoader)));
+            }
+            LoaderSource::Yaml(name) => {
+                loaders.push((name.clone(), Box::new(YamlLoader)));
+            }
+        }
+    }
 
     for search_dir in dir.ancestors() {
         for (file_name, loader) in &loaders {
@@ -64,3 +92,23 @@ where
 
     Ok(None)
 }
+
+pub fn search<T>(name: &str, dir: &Path) -> Result<Option<Config<T>>, ConfigError>
+where
+    T: for<'de> Deserialize<'de> + 'static,
+{
+    load(
+        dir,
+        &[
+            LoaderSource::PackageJson(name.to_string()),
+            LoaderSource::Json(format!(".{}rc", name)),
+            LoaderSource::Json(format!("{}.config.json", name)),
+            LoaderSource::Yaml(format!(".{}rc.yaml", name)),
+            LoaderSource::Yaml(format!(".{}rc.yml", name)),
+            LoaderSource::Js(format!(".{}.rc.js", name)),
+            LoaderSource::Js(format!(".{}.rc.cjs", name)),
+            LoaderSource::Js(format!("{}.config.js", name)),
+            LoaderSource::Js(format!("{}.config.cjs", name)),
+        ],
+    )
+}

compiler/crates/relay-bin/src/errors.rs

@@ -12,8 +12,8 @@ pub enum Error {
     #[error("Unable to run the relay language server. Error details: \n{details}")]
     LSPError { details: String },
 
-    #[error("Unable to initialize relay compiler configuration. Error details: \n{details}")]
-    ConfigError { details: String },
+    #[error("{0}")]
+    ConfigError(relay_compiler::errors::Error),
 
     #[error("Unable to run relay compiler. Error details: \n{details}")]
     CompilerError { details: String },

compiler/crates/relay-bin/src/main.rs

@@ -10,7 +10,8 @@ use common::ConsoleLogger;
 use log::{error, info};
 use relay_compiler::{
     build_project::artifact_writer::ArtifactValidationWriter, compiler::Compiler, config::Config,
-    FileSourceKind, LocalPersister, OperationPersister, PersistConfig, RemotePersister,
+    errors::Error as CompilerError, FileSourceKind, LocalPersister, OperationPersister,
+    PersistConfig, RemotePersister,
 };
 use relay_lsp::{start_language_server, DummyExtraDataProvider};
 use schema::SDLSchema;
@@ -158,21 +159,17 @@ async fn main() {
     match result {
         Ok(_) => info!("Done."),
         Err(err) => {
-            error!("{:?}", err);
+            error!("{}", err);
             std::process::exit(1);
         }
     }
 }
 
 fn get_config(config_path: Option<PathBuf>) -> Result<Config, Error> {
     match config_path {
-        Some(config_path) => Config::load(config_path).map_err(|err| Error::ConfigError {
-            details: format!("{:?}", err),
-        }),
+        Some(config_path) => Config::load(config_path).map_err(Error::ConfigError),
         None => Config::search(&current_dir().expect("Unable to get current working directory."))
-            .map_err(|err| Error::ConfigError {
-                details: format!("{:?}", err),
-            }),
+            .map_err(Error::ConfigError),
     }
 }
 
@@ -198,12 +195,12 @@ async fn handle_compiler_command(command: CompileCommand) -> Result<(), Error> {
     configure_logger(command.output, TerminalMode::Mixed);
 
     if command.cli_config.is_defined() {
-        return Err(Error::ConfigError {
+        return Err(Error::ConfigError(CompilerError::ConfigError {
             details: format!(
                 "\nPassing Relay compiler configuration is not supported. Please add `relay.config.json` file,\nor \"relay\" section to your `package.json` file.\n\nCompiler configuration JSON:{}",
                 command.cli_config.get_config_string(),
             ),
-        });
+        }));
     }
 
     let mut config = get_config(command.config)?;

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

@@ -20,6 +20,7 @@ use fnv::{FnvBuildHasher, FnvHashSet};
 use graphql_ir::{OperationDefinition, Program};
 use indexmap::IndexMap;
 use intern::string_key::{Intern, StringKey};
+use js_config_loader::LoaderSource;
 use log::warn;
 use persist_query::PersistError;
 use rayon::prelude::*;
@@ -35,6 +36,8 @@ use serde::de::Error as DeError;
 use serde::{Deserialize, Deserializer, Serialize};
 use serde_json::Value;
 use sha1::{Digest, Sha1};
+use std::env::current_dir;
+use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::{fmt, vec};
@@ -162,50 +165,82 @@ impl From<SingleProjectConfigFile> for Config {
 
 impl Config {
     pub fn search(start_dir: &Path) -> Result<Self> {
-        match js_config_loader::search("relay", start_dir) {
+        Self::load_config(
+            start_dir,
+            &[
+                LoaderSource::PackageJson("relay".to_string()),
+                LoaderSource::Json("relay.config.json".to_string()),
+                LoaderSource::Js("relay.config.js".to_string()),
+            ],
+        )
+    }
+
+    pub fn load(config_path: PathBuf) -> Result<Self> {
+        let loader = if config_path.extension() == Some(OsStr::new("js")) {
+            LoaderSource::Js(config_path.display().to_string())
+        } else if config_path.extension() == Some(OsStr::new("json")) {
+            LoaderSource::Json(config_path.display().to_string())
+        } else {
+            return Err(Error::ConfigError {
+                details: format!(
+                    "Invalid file extension. Expected `.js` or `.json`. Provided file \"{}\".",
+                    config_path.display()
+                ),
+            });
+        };
+        Self::load_config(
+            &current_dir().expect("Unable to get current working directory."),
+            &[loader],
+        )
+    }
+
+    fn load_config(start_dir: &Path, loaders_sources: &[LoaderSource]) -> Result<Self> {
+        match js_config_loader::load(start_dir, loaders_sources) {
             Ok(Some(config)) => Self::from_struct(config.path, config.value, true),
             Ok(None) => Err(Error::ConfigError {
-                details: "No config found.".to_string(),
+                details: format!(
+                    r#"
+Configuration for Relay compiler not found.
+
+Please make sure that the configuration file is created in {}.
+
+You can also pass the path to the configuration file as `relay-compiler ./path-to-config/relay.json`.
+
+Example file:
+{{
+  "src": "./src",
+  "schema": "./path-to/schema.graphql"
+}}
+"#,
+                    match loaders_sources.len() {
+                        1 => loaders_sources[0].to_string(),
+                        2 => format!("{} or {}", loaders_sources[0], loaders_sources[1]),
+                        _ => {
+                            let mut loaders_str = loaders_sources
+                                .iter()
+                                .map(|loader| loader.to_string())
+                                .collect::<Vec<_>>();
+                            let last_option = loaders_str.pop().unwrap();
+                            format!("{}, or {}", loaders_str.join(", "), last_option)
+                        }
+                    }
+                ),
             }),
             Err(error) => Err(Error::ConfigError {
                 details: format!("Error searching config: {}", error),
             }),
         }
     }
 
-    pub fn load(config_path: PathBuf) -> Result<Self> {
-        let config_string =
-            std::fs::read_to_string(&config_path).map_err(|err| Error::ConfigError {
-                details: format!(
-                    "Failed to read config file `{}`. {:?}",
-                    config_path.display(),
-                    err
-                ),
-            })?;
-        Self::from_string(config_path, &config_string, true)
-    }
-
     /// Loads a config file without validation for use in tests.
     #[cfg(test)]
     pub fn from_string_for_test(config_string: &str) -> Result<Self> {
-        Self::from_string(
-            "/virtual/root/virtual_config.json".into(),
-            config_string,
-            false,
-        )
-    }
-
-    /// `validate_fs` disables all filesystem checks for existence of files
-    fn from_string(config_path: PathBuf, config_string: &str, validate_fs: bool) -> Result<Self> {
+        let path = PathBuf::from("/virtual/root/virtual_config.json");
         let config_file: ConfigFile =
             serde_json::from_str(config_string).map_err(|err| Error::ConfigError {
-                details: format!(
-                    "Failed to parse config file `{}`: {}",
-                    config_path.display(),
-                    err,
-                ),
+                details: format!("Failed to parse config file `{}`: {}", path.display(), err,),
             })?;
-        Self::from_struct(config_path, config_file, validate_fs)
+        Self::from_struct(path, config_file, false)
     }
 
     /// `validate_fs` disables all filesystem checks for existence of files

packages/relay-compiler/README.md

@@ -34,22 +34,20 @@ Relay Compiler will automatically discover the config if:
   project (i.e. in the same folder as the `package.json` file).
 - The `package.json` file contains a `"relay"` key.
 
-Additionally, this config file can be specified with the CLI argument `--config`
-as follows:
+Alternatively, the path to a configuration file can be specified as an argument:
 
 ```shell
-npm run relay --config ./relay.json
+npm run relay ./relay.json
 ```
 
 or with yarn
 
 ```shell
-yarn relay --config ./relay.json
+yarn relay ./relay.json
 ```
 
-Please note, that if you pass configuration options via --cli arguments, you'll
-need to provide a separate configuration for the
-[babel plugin](https://www.npmjs.com/package/babel-plugin-relay).
+Please note, in this case you'll need to provide a separate configuration for
+the [babel plugin](https://www.npmjs.com/package/babel-plugin-relay).
 
 ## File Finder