Implements RFC 8, fixing #606

rustwasm · Jul 6, 2021 · d79e721 · d79e721
1 parent 260d79a
commit d79e721
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 9 deletions.
diff --git a/src/command/utils.rs b/src/command/utils.rs
@@ -37,6 +37,7 @@ fn find_manifest_from_cwd() -> Result<PathBuf, failure::Error> {
 
 /// Construct our `pkg` directory in the crate.
 pub fn create_pkg_dir(out_dir: &Path) -> Result<(), failure::Error> {
+    let _ = fs::remove_dir_all(&out_dir); // Clean up any existing directory and ignore errors
     fs::create_dir_all(&out_dir)?;
     fs::write(out_dir.join(".gitignore"), "*")?;
     Ok(())

diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs
@@ -8,8 +8,8 @@
 
 mod npm;
 
-use std::fs;
 use std::path::Path;
+use std::{collections::HashMap, fs};
 
 use self::npm::{
     repository::Repository, CommonJSPackage, ESModulesPackage, NoModulesPackage, NpmPackage,
@@ -575,14 +575,25 @@ impl CrateData {
         target: Target,
     ) -> Result<(), Error> {
         let pkg_file_path = out_dir.join("package.json");
+        // Check if a `package.json` was already generated by wasm-bindgen, if so
+        // we merge the NPM dependencies already specified in it.
+        let existing_deps = if pkg_file_path.exists() {
+            // It's just a map of dependency names to versions
+            Some(serde_json::from_str::<HashMap<String, String>>(
+                &fs::read_to_string(&pkg_file_path)?,
+            )?)
+        } else {
+            None
+        };
         let npm_data = match target {
-            Target::Nodejs => self.to_commonjs(scope, disable_dts, out_dir),
-            Target::NoModules => self.to_nomodules(scope, disable_dts, out_dir),
-            Target::Bundler => self.to_esmodules(scope, disable_dts, out_dir),
-            Target::Web => self.to_web(scope, disable_dts, out_dir),
+            Target::Nodejs => self.to_commonjs(scope, disable_dts, existing_deps, out_dir),
+            Target::NoModules => self.to_nomodules(scope, disable_dts, existing_deps, out_dir),
+            Target::Bundler => self.to_esmodules(scope, disable_dts, existing_deps, out_dir),
+            Target::Web => self.to_web(scope, disable_dts, existing_deps, out_dir),
         };
 
         let npm_json = serde_json::to_string_pretty(&npm_data)?;
+
         fs::write(&pkg_file_path, npm_json)
             .with_context(|_| format!("failed to write: {}", pkg_file_path.display()))?;
         Ok(())
@@ -657,7 +668,13 @@ impl CrateData {
         })
     }
 
-    fn to_commonjs(&self, scope: &Option<String>, disable_dts: bool, out_dir: &Path) -> NpmPackage {
+    fn to_commonjs(
+        &self,
+        scope: &Option<String>,
+        disable_dts: bool,
+        dependencies: Option<HashMap<String, String>>,
+        out_dir: &Path,
+    ) -> NpmPackage {
         let data = self.npm_data(scope, false, disable_dts, out_dir);
         let pkg = &self.data.packages[self.current_idx];
 
@@ -683,13 +700,15 @@ impl CrateData {
             homepage: data.homepage,
             types: data.dts_file,
             keywords: data.keywords,
+            dependencies,
         })
     }
 
     fn to_esmodules(
         &self,
         scope: &Option<String>,
         disable_dts: bool,
+        dependencies: Option<HashMap<String, String>>,
         out_dir: &Path,
     ) -> NpmPackage {
         let data = self.npm_data(scope, true, disable_dts, out_dir);
@@ -718,10 +737,17 @@ impl CrateData {
             types: data.dts_file,
             side_effects: false,
             keywords: data.keywords,
+            dependencies,
         })
     }
 
-    fn to_web(&self, scope: &Option<String>, disable_dts: bool, out_dir: &Path) -> NpmPackage {
+    fn to_web(
+        &self,
+        scope: &Option<String>,
+        disable_dts: bool,
+        dependencies: Option<HashMap<String, String>>,
+        out_dir: &Path,
+    ) -> NpmPackage {
         let data = self.npm_data(scope, false, disable_dts, out_dir);
         let pkg = &self.data.packages[self.current_idx];
 
@@ -748,13 +774,15 @@ impl CrateData {
             types: data.dts_file,
             side_effects: false,
             keywords: data.keywords,
+            dependencies,
         })
     }
 
     fn to_nomodules(
         &self,
         scope: &Option<String>,
         disable_dts: bool,
+        dependencies: Option<HashMap<String, String>>,
         out_dir: &Path,
     ) -> NpmPackage {
         let data = self.npm_data(scope, false, disable_dts, out_dir);
@@ -782,6 +810,7 @@ impl CrateData {
             homepage: data.homepage,
             types: data.dts_file,
             keywords: data.keywords,
+            dependencies,
         })
     }
 

diff --git a/src/manifest/npm/commonjs.rs b/src/manifest/npm/commonjs.rs
@@ -1,3 +1,5 @@
+use std::collections::HashMap;
+
 use manifest::npm::repository::Repository;
 
 #[derive(Serialize)]
@@ -21,4 +23,6 @@ pub struct CommonJSPackage {
     pub types: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub keywords: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub dependencies: Option<HashMap<String, String>>,
 }
diff --git a/src/manifest/npm/esmodules.rs b/src/manifest/npm/esmodules.rs
@@ -1,3 +1,5 @@
+use std::collections::HashMap;
+
 use manifest::npm::repository::Repository;
 
 #[derive(Serialize)]
@@ -23,4 +25,6 @@ pub struct ESModulesPackage {
     pub side_effects: bool,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub keywords: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub dependencies: Option<HashMap<String, String>>,
 }
diff --git a/src/manifest/npm/nomodules.rs b/src/manifest/npm/nomodules.rs
@@ -1,3 +1,5 @@
+use std::collections::HashMap;
+
 use manifest::npm::repository::Repository;
 
 #[derive(Serialize)]
@@ -21,4 +23,6 @@ pub struct NoModulesPackage {
     pub types: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub keywords: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub dependencies: Option<HashMap<String, String>>,
 }
diff --git a/tests/all/manifest.rs b/tests/all/manifest.rs
@@ -1,5 +1,5 @@
 use assert_cmd::prelude::*;
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::fs;
 use std::path::PathBuf;
 use utils::{self, fixture};
@@ -314,6 +314,54 @@ fn it_creates_a_package_json_with_correct_keys_when_types_are_skipped() {
     assert_eq!(actual_files, expected_files);
 }
 
+#[test]
+fn it_creates_a_package_json_with_npm_dependencies_provided_by_wasm_bindgen() {
+    let fixture = fixture::js_hello_world();
+    let out_dir = fixture.path.join("pkg");
+    let crate_data = manifest::CrateData::new(&fixture.path, None).unwrap();
+    wasm_pack::command::utils::create_pkg_dir(&out_dir).unwrap();
+    // Write a `package.json` in the out_dir, as wasm-bindgen does:
+    utils::manifest::create_wbg_package_json(
+        &out_dir,
+        r#"
+        { "foo": "^1.2.3" }
+    "#,
+    )
+    .unwrap();
+    assert!(crate_data
+        .write_package_json(&out_dir, &None, true, Target::Bundler)
+        .is_ok());
+    let package_json_path = &out_dir.join("package.json");
+    fs::metadata(package_json_path).unwrap();
+    utils::manifest::read_package_json(&fixture.path, &out_dir).unwrap();
+    let pkg = utils::manifest::read_package_json(&fixture.path, &out_dir).unwrap();
+    assert_eq!(pkg.name, "js-hello-world");
+    assert_eq!(pkg.repository.ty, "git");
+    assert_eq!(
+        pkg.repository.url,
+        "https://github.com/rustwasm/wasm-pack.git"
+    );
+    assert_eq!(pkg.module, "js_hello_world.js");
+
+    let actual_files: HashSet<String> = pkg.files.into_iter().collect();
+    let expected_files: HashSet<String> = [
+        "js_hello_world_bg.wasm",
+        "js_hello_world_bg.js",
+        "js_hello_world.js",
+    ]
+    .iter()
+    .map(|&s| String::from(s))
+    .collect();
+    assert_eq!(actual_files, expected_files);
+
+    let dependencies: Option<HashMap<String, String>> = pkg.dependencies;
+    assert!(dependencies.is_some());
+    let mut expected_dependencies: HashMap<String, String> = HashMap::new();
+    expected_dependencies.insert("foo".to_owned(), "^1.2.3".to_owned());
+
+    assert_eq!(dependencies.unwrap(), expected_dependencies);
+}
+
 #[test]
 fn it_errors_when_wasm_bindgen_is_not_declared() {
     let fixture = fixture::bad_cargo_toml();

diff --git a/tests/all/utils/manifest.rs b/tests/all/utils/manifest.rs
@@ -1,6 +1,6 @@
-use std::fs::File;
 use std::io::prelude::*;
 use std::path::Path;
+use std::{collections::HashMap, fs::File};
 
 use failure::Error;
 use serde_json;
@@ -25,6 +25,7 @@ pub struct NpmPackage {
     pub side_effects: bool,
     pub homepage: Option<String>,
     pub keywords: Option<Vec<String>>,
+    pub dependencies: Option<HashMap<String, String>>,
 }
 
 fn default_none() -> String {
@@ -50,3 +51,8 @@ pub fn read_package_json(path: &Path, out_dir: &Path) -> Result<NpmPackage, Erro
 
     Ok(serde_json::from_str(&pkg_contents)?)
 }
+
+pub fn create_wbg_package_json(out_dir: &Path, contents: &str) -> Result<(), Error> {
+    let manifest_path = out_dir.join("package.json");
+    Ok(std::fs::write(manifest_path, contents)?)
+}