Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support deno target #908

Closed
wants to merge 5 commits into from
Closed

Support deno target #908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4f1cc33
Support deno target
juzi5201314 Aug 27, 2020
b112eae
rustfmt
juzi5201314 Aug 27, 2020
c0cc625
Delete `*` in .gitignore of Deno target #909
juzi5201314 Aug 28, 2020
40660ac
Fix the error of `create_pkg_dir` in tests
juzi5201314 Aug 28, 2020
1c59725
Merge branch 'rustwasm:master' into master
juzi5201314 Sep 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/bindgen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ fn build_target_arg_legacy(target: Target, cli_path: &Path) -> Result<String, fa
}
}
Target::Bundler => "--browser",
Target::Deno => "--deno",
};
Ok(target_arg.to_string())
}
6 changes: 5 additions & 1 deletion src/command/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ pub enum Target {
/// in a browser but pollutes the global namespace and must be manually
/// instantiated.
NoModules,
/// Correspond to --target deno here the output is Modules available for deno
Deno,
}

impl Default for Target {
Expand All @@ -70,6 +72,7 @@ impl fmt::Display for Target {
Target::Web => "web",
Target::Nodejs => "nodejs",
Target::NoModules => "no-modules",
Target::Deno => "deno",
};
write!(f, "{}", s)
}
Expand All @@ -83,6 +86,7 @@ impl FromStr for Target {
"web" => Ok(Target::Web),
"nodejs" => Ok(Target::Nodejs),
"no-modules" => Ok(Target::NoModules),
"deno" => Ok(Target::Deno),
_ => bail!("Unknown target: {}", s),
}
}
Expand Down Expand Up @@ -329,7 +333,7 @@ impl Build {

fn step_create_dir(&mut self) -> Result<(), Error> {
info!("Creating a pkg directory...");
create_pkg_dir(&self.out_dir)?;
create_pkg_dir(&self.out_dir, &self.target)?;
info!("Created a pkg directory at {:#?}.", &self.crate_path);
Ok(())
}
Expand Down
10 changes: 8 additions & 2 deletions src/command/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::fs;
use std::path::{Path, PathBuf};
use std::time::Duration;
use walkdir::WalkDir;
use command::build::Target;

/// If an explicit path is given, then use it, otherwise assume the current
/// directory is the crate path.
Expand Down Expand Up @@ -36,9 +37,14 @@ 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> {
pub fn create_pkg_dir(out_dir: &Path, target: &Target) -> Result<(), failure::Error> {
fs::create_dir_all(&out_dir)?;
fs::write(out_dir.join(".gitignore"), "*")?;
let mut gitignore_contents = String::new();
match target {
Target::Deno => {}
_ => gitignore_contents.push_str("*")
}
fs::write(out_dir.join(".gitignore"), gitignore_contents)?;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of writing an empty file, it would be cleaner to not write it at all, I think.

But also, I'm nut sure this is necessary, why would targeting Deno be different in this regard? You might want to package up the resulting files into a separate repository (equivalent to putting it on npm for node), but at the same time want to keep the rust lib's source clean.

Having a wildcard .gitignore doesn't make sense in any other way, you obviously want to keep the results somewhere.

Copy link

@Mipsters Mipsters Feb 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was the default behavior, they just removed that option for deno
the deleted line in 41 wrote * to .gitignore in every case, but @juzi5201314 probably concluded that it is not relevant for deno
this is still the state in the main branch (as of 2nd Feb, 2022)
and in the latest release atm
I agree about preferring not to create the .gitignore at all, but keeping the * for all other targets is just keeping the same behavior
changing this behavior is outside of the scope of this pr :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we should leave create_pkg_dir as it is.
I could see one improvement to the status quo where we could read the current contents of an existing .gitignore before deleting the directory and then writing them back afterwards so modifications made by the user persist, but this should be a separate PR if it is a desired feature.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment is not very clear, reading it again now. I think my point was that this function should be left as-is, no need to special case it for Deno.

Ok(())
}

Expand Down
2 changes: 2 additions & 0 deletions src/manifest/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,8 @@ impl CrateData {
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),
// Deno does not need package.json
Target::Deno => return Ok(()),
};

let npm_json = serde_json::to_string_pretty(&npm_data)?;
Expand Down
26 changes: 13 additions & 13 deletions tests/all/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ fn it_creates_a_package_json_default_path() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &None, false, Target::Bundler)
.is_ok());
Expand Down Expand Up @@ -113,7 +113,7 @@ fn it_creates_a_package_json_provided_path() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &None, false, Target::Bundler)
.is_ok());
Expand Down Expand Up @@ -142,7 +142,7 @@ fn it_creates_a_package_json_provided_path_with_scope() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &Some("test".to_string()), false, Target::Bundler,)
.is_ok());
Expand Down Expand Up @@ -171,7 +171,7 @@ fn it_creates_a_pkg_json_with_correct_files_on_node() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &None, false, Target::Nodejs)
.is_ok());
Expand Down Expand Up @@ -205,7 +205,7 @@ fn it_creates_a_pkg_json_with_correct_files_on_nomodules() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &None, false, Target::NoModules)
.is_ok());
Expand Down Expand Up @@ -239,7 +239,7 @@ fn it_creates_a_package_json_with_correct_files_when_out_name_is_provided() {
let fixture = fixture::js_hello_world();
let out_dir = fixture.path.join("pkg");
let crate_data = manifest::CrateData::new(&fixture.path, Some("index".to_owned())).unwrap();
wasm_pack::command::utils::create_pkg_dir(&out_dir).unwrap();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &None, false, Target::Bundler)
.is_ok());
Expand Down Expand Up @@ -271,7 +271,7 @@ fn it_creates_a_pkg_json_in_out_dir() {
let fixture = fixture::js_hello_world();
let out_dir = fixture.path.join("./custom/out");
let crate_data = manifest::CrateData::new(&fixture.path, None).unwrap();
wasm_pack::command::utils::create_pkg_dir(&out_dir).unwrap();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &None, false, Target::Bundler)
.is_ok());
Expand All @@ -286,7 +286,7 @@ fn it_creates_a_package_json_with_correct_keys_when_types_are_skipped() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
assert!(crate_data
.write_package_json(&out_dir, &None, true, Target::Bundler)
.is_ok());
Expand Down Expand Up @@ -351,7 +351,7 @@ fn it_sets_homepage_field_if_available_in_cargo_toml() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
crate_data
.write_package_json(&out_dir, &None, true, Target::Bundler)
.unwrap();
Expand All @@ -367,7 +367,7 @@ fn it_sets_homepage_field_if_available_in_cargo_toml() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
crate_data
.write_package_json(&out_dir, &None, true, Target::Bundler)
.unwrap();
Expand Down Expand Up @@ -406,7 +406,7 @@ fn it_sets_keywords_field_if_available_in_cargo_toml() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
crate_data
.write_package_json(&out_dir, &None, true, Target::Bundler)
.unwrap();
Expand All @@ -424,7 +424,7 @@ fn it_sets_keywords_field_if_available_in_cargo_toml() {
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();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
crate_data
.write_package_json(&out_dir, &None, true, Target::Bundler)
.unwrap();
Expand Down Expand Up @@ -524,7 +524,7 @@ fn it_lists_license_files_in_files_field_of_package_json() {

let crate_data = manifest::CrateData::new(&fixture.path, None).unwrap();

wasm_pack::command::utils::create_pkg_dir(&out_dir).unwrap();
wasm_pack::command::utils::create_pkg_dir(&out_dir, &Target::NoModules).unwrap();
license::copy_from_crate(&crate_data, &fixture.path, &out_dir).unwrap();
crate_data
.write_package_json(&out_dir, &None, false, Target::Bundler)
Expand Down