From 8e52d3bb2e0709f9104ed10f2a04da4ca0e582b6 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Mon, 28 Oct 2019 15:34:53 -0500 Subject: [PATCH 01/18] Add TODO for preview docs --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index ba92aadc4..379f9c80b 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,8 @@ If you would like to be able to publish your code to multiple places, please see Additionally, you can preview different environments. This is useful if you have different builds for different environments (like staging vs. production), but typically isn't needed. For more information see the [environments documentation](https://github.com/cloudflare/wrangler/blob/master/docs/content/environments.md). + + - ### 🗂️ `kv` Interact with your Cloudflare Workers KV store. [Check out the docs.](./docs/content/kv_commands.md) From 6062adf997f1d1168c7052e3b857d20543d44fbe Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Mon, 28 Oct 2019 15:33:04 -0500 Subject: [PATCH 02/18] Add the ability to preview without opening the browser --- src/commands/publish/preview/mod.rs | 15 ++++++++++----- src/main.rs | 10 +++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/commands/publish/preview/mod.rs b/src/commands/publish/preview/mod.rs index 0ba514588..b8a2adfb7 100644 --- a/src/commands/publish/preview/mod.rs +++ b/src/commands/publish/preview/mod.rs @@ -34,6 +34,7 @@ pub fn preview( body: Option, livereload: bool, verbose: bool, + browser: bool, ) -> Result<(), failure::Error> { let sites_preview: bool = target.site.is_some(); @@ -51,10 +52,12 @@ pub fn preview( info!("Opened websocket server on port {}", ws_port); - open_browser(&format!( + if browser { + open_browser(&format!( "https://cloudflareworkers.com/?wrangler_session_id={0}&wrangler_ws_port={1}&hide_editor#{2}:{3}{4}", &session.to_string(), ws_port, script_id, https_str, preview_host, ))?; + } //don't do initial GET + POST with livereload as the expected behavior is unclear. @@ -68,10 +71,12 @@ pub fn preview( verbose, )?; } else { - open_browser(&format!( - "https://cloudflareworkers.com/?hide_editor#{0}:{1}{2}", - script_id, https_str, preview_host - ))?; + if browser { + open_browser(&format!( + "https://cloudflareworkers.com/?hide_editor#{0}:{1}{2}", + script_id, https_str, preview_host + ))?; + } let cookie = format!( "__ew_fiddle_preview={}{}{}{}", diff --git a/src/main.rs b/src/main.rs index 70103aa6d..af37b36a0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -307,6 +307,13 @@ fn run() -> Result<(), failure::Error> { "{} Preview your code temporarily on cloudflareworkers.com", emoji::MICROSCOPE )) + .arg( + Arg::with_name("terminal") + .help("Preview your worker in the terminal and not the browser") + .short("t") + .long("terminal") + .takes_value(false) + ) .arg( Arg::with_name("method") .help("Type of request to preview your worker with (get, post)") @@ -471,8 +478,9 @@ fn run() -> Result<(), failure::Error> { let watch = matches.is_present("watch"); let verbose = matches.is_present("verbose"); + let browser = !matches.is_present("terminal"); - commands::preview(target, user, method, body, watch, verbose)?; + commands::preview(target, user, method, body, watch, verbose, browser)?; } else if matches.subcommand_matches("whoami").is_some() { log::info!("Getting User settings"); let user = settings::global_user::GlobalUser::new()?; From 05f340fa37d96c365f6419f1b342f17d7a4a6f46 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Mon, 28 Oct 2019 15:39:53 -0500 Subject: [PATCH 03/18] Don't open the browser when testing wrangler preview --- tests/preview.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/preview.rs b/tests/preview.rs index fd3aeeb94..daa492cda 100644 --- a/tests/preview.rs +++ b/tests/preview.rs @@ -72,5 +72,5 @@ fn preview(fixture: &str) { let mut preview = Command::cargo_bin(env!("CARGO_PKG_NAME")).unwrap(); preview.current_dir(utils::fixture_path(fixture)); - preview.arg("preview").assert().success(); + preview.arg("preview").arg("-t").assert().success(); } From 9106cf8afdf14e670af6ab202b8c822c000fb14b Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Fri, 1 Nov 2019 10:57:23 -0500 Subject: [PATCH 04/18] Switch from --terminal to --headless --- src/commands/init/subcommand.rs | 30 ------------------------------ src/commands/preview/mod.rs | 6 +++--- src/main.rs | 11 +++++------ tests/preview.rs | 2 +- 4 files changed, 9 insertions(+), 40 deletions(-) delete mode 100644 src/commands/init/subcommand.rs diff --git a/src/commands/init/subcommand.rs b/src/commands/init/subcommand.rs deleted file mode 100644 index 81e1e4ab4..000000000 --- a/src/commands/init/subcommand.rs +++ /dev/null @@ -1,30 +0,0 @@ -use clap::{App, Arg, SubCommand}; - -use crate::terminal::emoji; - -pub fn get() -> App { - SubCommand::with_name("init") - .about(&*format!( - "{} Create a wrangler.toml for an existing project", - emoji::INBOX - )) - .arg( - Arg::with_name("name") - .help("the name of your worker! defaults to 'worker'") - .index(1), - ) - .arg( - Arg::with_name("type") - .short("t") - .long("type") - .takes_value(true) - .help("the type of project you want generated"), - ) - .arg( - Arg::with_name("site") - .short("s") - .long("site") - .takes_value(false) - .help("initializes a Workers Sites project. Overrides `type` and `template`"), - ) -} diff --git a/src/commands/preview/mod.rs b/src/commands/preview/mod.rs index 99c0f796b..74a84f457 100644 --- a/src/commands/preview/mod.rs +++ b/src/commands/preview/mod.rs @@ -34,7 +34,7 @@ pub fn preview( body: Option, livereload: bool, verbose: bool, - browser: bool, + headless: bool, ) -> Result<(), failure::Error> { commands::build(&target)?; @@ -54,7 +54,7 @@ pub fn preview( info!("Opened websocket server on port {}", ws_port); - if browser { + if !headless { open_browser(&format!( "https://cloudflareworkers.com/?wrangler_session_id={0}&wrangler_ws_port={1}&hide_editor#{2}:{3}{4}", &session.to_string(), ws_port, script_id, https_str, preview_host, @@ -73,7 +73,7 @@ pub fn preview( verbose, )?; } else { - if browser { + if !headless { open_browser(&format!( "https://cloudflareworkers.com/?hide_editor#{0}:{1}{2}", script_id, https_str, preview_host diff --git a/src/main.rs b/src/main.rs index 40a4f16e5..95a4da0e6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -309,10 +309,9 @@ fn run() -> Result<(), failure::Error> { emoji::MICROSCOPE )) .arg( - Arg::with_name("terminal") - .help("Preview your worker in the terminal and not the browser") - .short("t") - .long("terminal") + Arg::with_name("headless") + .help("Don't open the browser on preview") + .long("headless") .takes_value(false) ) .arg( @@ -479,9 +478,9 @@ fn run() -> Result<(), failure::Error> { let watch = matches.is_present("watch"); let verbose = matches.is_present("verbose"); - let browser = !matches.is_present("terminal"); + let headless = matches.is_present("headless"); - commands::preview(target, user, method, body, watch, verbose, browser)?; + commands::preview(target, user, method, body, watch, verbose, headless)?; } else if matches.subcommand_matches("whoami").is_some() { log::info!("Getting User settings"); let user = settings::global_user::GlobalUser::new()?; diff --git a/tests/preview.rs b/tests/preview.rs index daa492cda..296228e1d 100644 --- a/tests/preview.rs +++ b/tests/preview.rs @@ -72,5 +72,5 @@ fn preview(fixture: &str) { let mut preview = Command::cargo_bin(env!("CARGO_PKG_NAME")).unwrap(); preview.current_dir(utils::fixture_path(fixture)); - preview.arg("preview").arg("-t").assert().success(); + preview.arg("preview").arg("--headless").assert().success(); } From 8f5fda2ed50b09c16adfff29491cf46b5bf5d5e4 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 10:31:00 -0600 Subject: [PATCH 05/18] Add link to API Token creation --- src/main.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 6dcaea294..4ccb17868 100644 --- a/src/main.rs +++ b/src/main.rs @@ -403,8 +403,9 @@ fn run() -> Result<(), failure::Error> { let default = !matches.is_present("api-key"); let user: GlobalUser = if default { - // Default: use API token. - message::info("Looking to use a Global API Key and email instead? Run \"wrangler config --api-key\". (Not Recommended)"); + // API Tokens are the default + message::info("To find your API token, go to https://dash.cloudflare.com/profile/api-tokens and create it using the \"Edit Cloudflare Workers\" template"); + message::info("If you are trying to use your Global API Key instead of an API Token (Not Recommended), run \"wrangler config --api-key\"."); println!("Enter API token: "); let mut api_token: String = read!("{}\n"); api_token.truncate(api_token.trim_end().len()); From cb23be8d1bd8b1041ee23a863cfcb1bf7804b013 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 13:50:50 -0600 Subject: [PATCH 06/18] Audit comments --- npm/install-wrangler.js | 1 - src/commands/build/watch/mod.rs | 4 ++-- src/commands/build/watch/watcher.rs | 2 +- src/commands/build/wranglerjs/mod.rs | 16 ++++++++-------- src/commands/build/wranglerjs/output.rs | 6 +++--- src/commands/config/mod.rs | 3 +-- src/commands/kv/bucket/upload.rs | 2 +- src/commands/kv/key/get.rs | 2 +- src/commands/kv/key/put.rs | 2 +- src/commands/mod.rs | 2 +- src/commands/preview/fiddle_messenger.rs | 8 ++++---- src/commands/preview/mod.rs | 5 +++-- src/commands/publish/upload_form/mod.rs | 2 +- src/main.rs | 2 +- src/settings/target/site.rs | 3 +++ src/settings/target/target.rs | 3 --- src/util.rs | 8 ++++---- wranglerjs/index.js | 2 +- 18 files changed, 36 insertions(+), 37 deletions(-) diff --git a/npm/install-wrangler.js b/npm/install-wrangler.js index e8a7932eb..215a8cc62 100755 --- a/npm/install-wrangler.js +++ b/npm/install-wrangler.js @@ -2,7 +2,6 @@ const axios = require("axios"); const os = require("os"); const { join, resolve } = require("path"); const { mkdirSync, existsSync } = require("fs"); -// while recent versions of Node can do that natively, wait until we can use it. const rimraf = require("rimraf"); const tar = require("tar"); const { get } = axios; diff --git a/src/commands/build/watch/mod.rs b/src/commands/build/watch/mod.rs index 148eb6ba7..c30fe05d0 100644 --- a/src/commands/build/watch/mod.rs +++ b/src/commands/build/watch/mod.rs @@ -20,8 +20,8 @@ const RUST_PATH: &str = "./"; // Paths to ignore live watching in Rust Workers const RUST_IGNORE: &'static [&str] = &["pkg", "target", "worker/generated"]; -/// watch a project for changes and re-build it when necessary, -/// outputting a build event to tx. +// watch a project for changes and re-build it when necessary, +// outputting a build event to tx. pub fn watch_and_build( target: &Target, tx: Option>, diff --git a/src/commands/build/watch/watcher.rs b/src/commands/build/watch/watcher.rs index 49f260df1..01c95122d 100644 --- a/src/commands/build/watch/watcher.rs +++ b/src/commands/build/watch/watcher.rs @@ -8,7 +8,7 @@ use failure::{format_err, Error}; use crate::terminal::message; use log::info; -/// Add cooldown for all types of events to watching logic +// Add cooldown for all types of events to watching logic pub fn wait_for_changes( rx: &Receiver, cooldown: Duration, diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index af6919fa9..e92607888 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -30,8 +30,8 @@ use std::thread; use std::time::Duration; // Run the underlying {wranglerjs} executable. -// -// In Rust we create a virtual file, pass the pass to {wranglerjs}, run the + +// In Rust we create a virtual file, pass it to {wranglerjs}, run the // executable and wait for completion. The file will receive the a serialized // {WranglerjsOutput} struct. // Note that the ability to pass a fd is platform-specific @@ -62,7 +62,7 @@ pub fn run_build_and_watch(target: &Target, tx: Option>) -> Result<() info!("Running {:?} in watch mode", command); - //Turbofish the result of the closure so we can use ? + // Turbofish the result of the closure so we can use ? thread::spawn::<_, Result<(), failure::Error>>(move || { let _command_guard = util::GuardedCommand::spawn(command); @@ -93,8 +93,8 @@ pub fn run_build_and_watch(target: &Target, tx: Option>) -> Result<() if is_first { is_first = false; message::info("Ignoring stale first change"); - //skip the first change event - //so we don't do a refresh immediately + // skip the first change event + // so we don't do a refresh immediately continue; } @@ -157,7 +157,8 @@ fn setup_build(target: &Target) -> Result<(Command, PathBuf, Bundle), failure::E let wranglerjs_path = install().expect("could not install wranglerjs"); command.arg(wranglerjs_path); - //put path to our wasm_pack as env variable so wasm-pack-plugin can utilize it + // export WASM_PACK_PATH for use by wasm-pack-plugin + // https://github.com/wasm-tool/wasm-pack-plugin/blob/caca20df84782223f002735a8a2e99b2291f957c/plugin.js#L13 let wasm_pack_path = install::install("wasm-pack", "rustwasm")?.binary("wasm-pack")?; command.env("WASM_PACK_PATH", wasm_pack_path); @@ -230,7 +231,6 @@ fn build_with_default_webpack( pub fn scaffold_site_worker(target: &Target) -> Result<(), failure::Error> { let build_dir = target.build_dir()?; - // TODO: this is a placeholder template. Replace with The Real Thing on launch. let template = "https://github.com/cloudflare/worker-sites-init"; if !Path::new(&build_dir).exists() { @@ -268,7 +268,7 @@ fn run_npm_install(dir: &PathBuf) -> Result<(), failure::Error> { info!("skipping npm install because node_modules exists"); } - // TODO(sven): figure out why the file doesn't exits in some cases? + // TODO: (sven) figure out why the file doesn't exist in some cases if flock_path.exists() { fs::remove_file(&flock_path)?; } diff --git a/src/commands/build/wranglerjs/output.rs b/src/commands/build/wranglerjs/output.rs index 5cd70de03..1e5f88ce4 100644 --- a/src/commands/build/wranglerjs/output.rs +++ b/src/commands/build/wranglerjs/output.rs @@ -6,7 +6,7 @@ use serde::Deserialize; use std::io::prelude::*; // This structure represents the communication between {wranglerjs} and -// {wrangler}. It is send back after {wranglerjs} completion. +// {wrangler}. It is sent back after {wranglerjs} completion. // FIXME(sven): make this private #[derive(Deserialize, Debug)] pub struct WranglerjsOutput { @@ -28,7 +28,7 @@ impl WranglerjsOutput { fn project_size_bytes(&self) -> u64 { let mut e = ZlibEncoder::new(Vec::new(), Compression::default()); - //approximation of how projects are gzipped + // approximation of how projects are gzipped e.write_all(&self.script.as_bytes()) .expect("could not write script buffer"); @@ -42,7 +42,7 @@ impl WranglerjsOutput { fn project_size_message(compressed_size: u64) -> String { const MAX_PROJECT_SIZE: u64 = 1 << 20; // 1 MiB - const WARN_THRESHOLD: u64 = MAX_PROJECT_SIZE - 81_920; //Warn when less than 80 KiB left to grow, ~92% usage + const WARN_THRESHOLD: u64 = MAX_PROJECT_SIZE - 81_920; // Warn when less than 80 KiB left to grow, ~92% usage const MAX_BEFORE_WARN: u64 = WARN_THRESHOLD - 1; let bytes_left = MAX_PROJECT_SIZE.checked_sub(compressed_size); diff --git a/src/commands/config/mod.rs b/src/commands/config/mod.rs index 944685740..e5b75e534 100644 --- a/src/commands/config/mod.rs +++ b/src/commands/config/mod.rs @@ -7,8 +7,7 @@ use std::path::PathBuf; use crate::settings::global_user::{get_global_config_dir, GlobalUser}; -// set the permissions on the dir, we want to avoid that other user reads to -// file +// set the permissions on the dir, we want to avoid other user reads of the file #[cfg(not(target_os = "windows"))] pub fn set_file_mode(file: &PathBuf) { File::open(&file) diff --git a/src/commands/kv/bucket/upload.rs b/src/commands/kv/bucket/upload.rs index b6a5c86e8..ce364b46a 100644 --- a/src/commands/kv/bucket/upload.rs +++ b/src/commands/kv/bucket/upload.rs @@ -172,7 +172,7 @@ mod tests { // Ensure the expected key and value was returned in the filtered pair list // Awkward field-by-field comparison below courtesy of not yet implementing // PartialEq for KeyValuePair in cloudflare-rs :) - // todo(gabbi): Implement PartialEq for KeyValuePair in cloudflare-rs. + // TODO: (gabbi) Implement PartialEq for KeyValuePair in cloudflare-rs. assert!(pair.key == actual[idx].key); assert!(pair.value == actual[idx].value); idx += 1; diff --git a/src/commands/kv/key/get.rs b/src/commands/kv/key/get.rs index 5ab68d363..094dba3e9 100644 --- a/src/commands/kv/key/get.rs +++ b/src/commands/kv/key/get.rs @@ -1,4 +1,4 @@ -// todo(gabbi): This file should use cloudflare-rs instead of our http::auth_client +// TODO:(gabbi) This file should use cloudflare-rs instead of our http::auth_client // when https://github.com/cloudflare/cloudflare-rs/issues/26 is handled (this is // because the GET key operation doesn't return json on success--just the raw // value). diff --git a/src/commands/kv/key/put.rs b/src/commands/kv/key/put.rs index 3502a8966..2c3cf5297 100644 --- a/src/commands/kv/key/put.rs +++ b/src/commands/kv/key/put.rs @@ -1,4 +1,4 @@ -// todo(gabbi): This file should use cloudflare-rs instead of our http::auth_client +// TODO: (gabbi) This file should use cloudflare-rs instead of our http::auth_client // when https://github.com/cloudflare/cloudflare-rs/issues/26 is handled (this is // because the SET key request body is not json--it is the raw value). diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 610bf8027..6acaba4d3 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -22,7 +22,7 @@ pub use subdomain::get_subdomain; pub use subdomain::set_subdomain; pub use whoami::whoami; -/// Run the given command and return its stdout. +// Run the given command and return its stdout. pub fn run(mut command: Command, command_name: &str) -> Result<(), failure::Error> { log::info!("Running {:?}", command); diff --git a/src/commands/preview/fiddle_messenger.rs b/src/commands/preview/fiddle_messenger.rs index c5a9b8e05..3beb1ee31 100644 --- a/src/commands/preview/fiddle_messenger.rs +++ b/src/commands/preview/fiddle_messenger.rs @@ -35,23 +35,23 @@ impl Handler for FiddleMessageServer { const SAFE_ADDRS: &[&str] = &["127.0.0.1", "localhost", "::1"]; - //origin() returns Result> + // origin() returns Result> let origin = handshake .request .origin()? .unwrap_or("unknown") .trim_end_matches(|c: char| c == '/' || c == ':' || c.is_numeric()); - //remote_addr returns Result> + // remote_addr returns Result> let incoming_addr = handshake.remote_addr()?; let incoming_addr = incoming_addr.as_ref().map_or("unknown", String::as_str); - //only allow connections from cloudflareworkers.com + // only allow connections from cloudflareworkers.com let origin_is_safe = SAFE_ORIGINS .iter() .any(|safe_origin| &origin == safe_origin); - //only allow incoming websocket connections from localhost/current machine. + // only allow incoming websocket connections from localhost/current machine. let addr_is_safe = SAFE_ADDRS .iter() .any(|safe_addr| &incoming_addr == safe_addr); diff --git a/src/commands/preview/mod.rs b/src/commands/preview/mod.rs index 74a84f457..7cb6416bd 100644 --- a/src/commands/preview/mod.rs +++ b/src/commands/preview/mod.rs @@ -48,7 +48,8 @@ pub fn preview( let https_str = if https { "https://" } else { "http://" }; if livereload { - let server = WebSocket::new(|out| FiddleMessageServer { out })?.bind("127.0.0.1:0")?; //explicitly use 127.0.0.1, since localhost can resolve to 2 addresses + // explicitly use 127.0.0.1, since localhost can resolve to 2 addresses + let server = WebSocket::new(|out| FiddleMessageServer { out })?.bind("127.0.0.1:0")?; let ws_port = server.local_addr()?.port(); @@ -61,7 +62,7 @@ pub fn preview( ))?; } - //don't do initial GET + POST with livereload as the expected behavior is unclear. + // don't do initial GET + POST with livereload as the expected behavior is unclear. let broadcaster = server.broadcaster(); thread::spawn(move || server.run()); diff --git a/src/commands/publish/upload_form/mod.rs b/src/commands/publish/upload_form/mod.rs index 5faebc0af..25fb68292 100644 --- a/src/commands/publish/upload_form/mod.rs +++ b/src/commands/publish/upload_form/mod.rs @@ -57,7 +57,7 @@ pub fn build( } TargetType::Webpack => { log::info!("Webpack project detected. Publishing..."); - // FIXME(sven): shouldn't new + // TODO: (sven) shouldn't new let build_dir = target.build_dir()?; let bundle = wranglerjs::Bundle::new(&build_dir); diff --git a/src/main.rs b/src/main.rs index 581166f2a..eec70e1dc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -564,7 +564,7 @@ fn run() -> Result<(), failure::Error> { } None => delete_matches .value_of("namespace-id") - .unwrap() // clap configs ensure that if "binding" isn't present,"namespace-id" must be. + .unwrap() // clap configs ensure that if "binding" isn't present, "namespace-id" must be. .to_string(), }; commands::kv::namespace::delete(&target, &user, &namespace_id)?; diff --git a/src/settings/target/site.rs b/src/settings/target/site.rs index 39e0ebefa..46f98a5ce 100644 --- a/src/settings/target/site.rs +++ b/src/settings/target/site.rs @@ -21,6 +21,9 @@ impl Site { site } + // if the user has configured `site.entry-point`, use that + // as the build directory. Otherwise use the default const + // SITE_ENTRY_POINT pub fn build_dir(&self, current_dir: PathBuf) -> Result { Ok(current_dir.join( self.entry_point diff --git a/src/settings/target/target.rs b/src/settings/target/target.rs index 38ae9e6d8..42dc49e64 100644 --- a/src/settings/target/target.rs +++ b/src/settings/target/target.rs @@ -40,9 +40,6 @@ impl Target { let current_dir = env::current_dir()?; // if `site` is configured, we want to isolate worker code // and build artifacts away from static site application code. - // if the user has configured `site.entry-point`, use that - // as the build directory. Otherwise use the default const - // SITE_BUILD_DIR match &self.site { Some(site_config) => site_config.build_dir(current_dir), None => Ok(current_dir), diff --git a/src/util.rs b/src/util.rs index 90a2ce2a6..2c72b40a5 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,9 +1,9 @@ use std::process::{Child, Command}; -/// wrapper around spawning child processes such that they -/// have the same behavior as spawned threads i.e. a spawned -/// child process using GuardedChild has the same lifetime as -/// the main thread. +// wrapper around spawning child processes such that they +// have the same behavior as spawned threads i.e. a spawned +// child process using GuardedChild has the same lifetime as +// the main thread. pub struct GuardedCommand(Child); impl GuardedCommand { diff --git a/wranglerjs/index.js b/wranglerjs/index.js index 2468863d4..2e81bfed2 100644 --- a/wranglerjs/index.js +++ b/wranglerjs/index.js @@ -34,7 +34,7 @@ function filterByExtension(ext) { } // Check if the config is a function and await it either way in - // case the result is a promise + // case the result is a Promise config = await (typeof config === "function" ? config({}) : config); if (Array.isArray(config)) { From c21702294e9c18417f5033a7c72aff753000b649 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 15:22:27 -0600 Subject: [PATCH 07/18] FIXME -> TODO Co-Authored-By: Ashley Michal --- src/commands/build/wranglerjs/output.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/build/wranglerjs/output.rs b/src/commands/build/wranglerjs/output.rs index 1e5f88ce4..87d401a13 100644 --- a/src/commands/build/wranglerjs/output.rs +++ b/src/commands/build/wranglerjs/output.rs @@ -7,7 +7,7 @@ use std::io::prelude::*; // This structure represents the communication between {wranglerjs} and // {wrangler}. It is sent back after {wranglerjs} completion. -// FIXME(sven): make this private +// TODO: (sven) make this private #[derive(Deserialize, Debug)] pub struct WranglerjsOutput { pub wasm: Option, From aa5209922ccfb0adcb2ec9c8800b2ca5a4228253 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 15:33:22 -0600 Subject: [PATCH 08/18] remove the from comment --- src/commands/build/wranglerjs/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index e92607888..706433e87 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -32,7 +32,7 @@ use std::time::Duration; // Run the underlying {wranglerjs} executable. // In Rust we create a virtual file, pass it to {wranglerjs}, run the -// executable and wait for completion. The file will receive the a serialized + // executable and wait for completion. The file will receive a serialized // {WranglerjsOutput} struct. // Note that the ability to pass a fd is platform-specific pub fn run_build(target: &Target) -> Result<(), failure::Error> { From f5fb490f1b23c7146e4a31cf5892f1a8f8d0e876 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 16:54:13 -0600 Subject: [PATCH 09/18] fix spacing in comment Co-Authored-By: Ashley Michal --- src/commands/build/wranglerjs/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index 706433e87..12f58dd7c 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -32,7 +32,7 @@ use std::time::Duration; // Run the underlying {wranglerjs} executable. // In Rust we create a virtual file, pass it to {wranglerjs}, run the - // executable and wait for completion. The file will receive a serialized +// executable and wait for completion. The file will receive a serialized // {WranglerjsOutput} struct. // Note that the ability to pass a fd is platform-specific pub fn run_build(target: &Target) -> Result<(), failure::Error> { From 49dc0fe31b058bb524e53dd59bd8aff37357a9d3 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 17:20:08 -0600 Subject: [PATCH 10/18] Replace sven todo with issue link --- src/commands/publish/upload_form/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/publish/upload_form/mod.rs b/src/commands/publish/upload_form/mod.rs index 25fb68292..b9527e4d1 100644 --- a/src/commands/publish/upload_form/mod.rs +++ b/src/commands/publish/upload_form/mod.rs @@ -57,7 +57,7 @@ pub fn build( } TargetType::Webpack => { log::info!("Webpack project detected. Publishing..."); - // TODO: (sven) shouldn't new + // TODO: https://github.com/cloudflare/wrangler/issues/850 let build_dir = target.build_dir()?; let bundle = wranglerjs::Bundle::new(&build_dir); From e6234e6f808acc78fc3d5274eeba98e4c94bda5d Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Wed, 6 Nov 2019 11:26:59 -0600 Subject: [PATCH 11/18] Update labels in CONTRIBUTING --- CONTRIBUTING.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f45d26d86..c3688ed46 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,11 +18,11 @@ Within 3 days, any incoming issue should be triaged. Triage involves: ### Labelling -- label all issues coming from non-team members with User Report -- labelling the category of the issue: Feature, External Bug, Bug, Maintenance, Docs -- optionally labelling a secondary category: Webpack, Routes, Workers Runtime, Refactor -- labelling the status of the issue: Need More Info, Needs Repro, Needs Design, PR Welcome -- optionally labelling other calls to action: Help Wanted, Question +- label all issues coming from non-team members with `user report` +- labelling the category of the issue: `feature`, `external bug`, `bug`, `maintenance`, `docs`, `refactor`, `release` +- labelling the status of the issue: `needs design`, `needs docs`, `needs more info`, `needs repro`, `needs template`, `PR attached`, `PR welcome`, `waiting on response` +- optionally labelling a subject: `cargo install`, `kv`, `routes`, `site`, `webpack`, `workers runtime` +- optionally labelling other calls to action: `help wanted`, `question`, `good first issue` ### Assignment @@ -36,7 +36,7 @@ our plans for the milestones and releases. ### Labelling -- labelling the priority of the issue: Critical, Nice to Have, Low Priority +- labelling the priority of the issue: `critical`, `nice to have`, `low priority` - labelling the status of the issue: Needs Design, PR Welcome ### Assignment and Milestones @@ -51,12 +51,12 @@ should be triaged immediately upon open by the PR author. ### Labelling -- All work-in-progress PRs should be labelled Work In Progress and the title should be +- All work-in-progress PRs should be labelled `work in progress` and the title should be annotated [WIP] for easy scanning. No WIP PRs will be reviewed until the annotations are removed. -- All PRs that need to be reviewed should be labelled Needs Review until they have +- All PRs that need to be reviewed should be labelled `needs review` until they have received all required reviews. -- All PRs should be labelled with a changelog label: BREAKING, Feature, Bug, Maintenance, Docs +- All PRs should be labelled with a changelog label: `BREAKING`, `feature`, `fix`, `maintenance`, `docs` ### Merging From 412079e82c58e475afb00638edd0e4eb9af39eed Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Wed, 6 Nov 2019 11:27:42 -0600 Subject: [PATCH 12/18] Webpack -> webpack --- src/commands/publish/upload_form/mod.rs | 2 +- src/main.rs | 2 +- src/settings/target/manifest.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/commands/publish/upload_form/mod.rs b/src/commands/publish/upload_form/mod.rs index b9527e4d1..e9c7a7b19 100644 --- a/src/commands/publish/upload_form/mod.rs +++ b/src/commands/publish/upload_form/mod.rs @@ -56,7 +56,7 @@ pub fn build( build_form(&assets) } TargetType::Webpack => { - log::info!("Webpack project detected. Publishing..."); + log::info!("webpack project detected. Publishing..."); // TODO: https://github.com/cloudflare/wrangler/issues/850 let build_dir = target.build_dir()?; let bundle = wranglerjs::Bundle::new(&build_dir); diff --git a/src/main.rs b/src/main.rs index eec70e1dc..6b7c593d2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -467,7 +467,7 @@ fn run() -> Result<(), failure::Error> { let name = matches.value_of("name"); let site = matches.is_present("site"); let target_type = if site { - // Workers Sites projects are always Webpack for now + // Workers Sites projects are always webpack for now Some(TargetType::Webpack) } else { match matches.value_of("type") { diff --git a/src/settings/target/manifest.rs b/src/settings/target/manifest.rs index 676498f60..5e8bf7cbb 100644 --- a/src/settings/target/manifest.rs +++ b/src/settings/target/manifest.rs @@ -122,7 +122,7 @@ impl Manifest { } pub fn get_target(&self, environment_name: Option<&str>) -> Result { - // Site projects are always Webpack for now; don't let toml override this. + // Site projects are always webpack for now; don't let toml override this. let target_type = match self.site { Some(_) => TargetType::Webpack, None => self.target_type.clone(), From be310df525902d431576b956c59c5e3a00fcca36 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 14:30:42 -0600 Subject: [PATCH 13/18] Require webpack_config to build with custom configuration --- src/commands/build/wranglerjs/bundle.rs | 4 --- src/commands/build/wranglerjs/mod.rs | 37 ++++++++++++------------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/commands/build/wranglerjs/bundle.rs b/src/commands/build/wranglerjs/bundle.rs index 1aeaec7ff..97b7f01ff 100644 --- a/src/commands/build/wranglerjs/bundle.rs +++ b/src/commands/build/wranglerjs/bundle.rs @@ -55,10 +55,6 @@ impl Bundle { self.wasm_path().exists() } - pub fn has_webpack_config(&self, webpack_config_path: &PathBuf) -> bool { - webpack_config_path.exists() - } - pub fn get_wasm_binding(&self) -> String { "wasm".to_string() } diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index 12f58dd7c..989af89ea 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -176,29 +176,28 @@ fn setup_build(target: &Target) -> Result<(Command, PathBuf, Bundle), failure::E command.arg(format!("--wasm-binding={}", bundle.get_wasm_binding())); - let webpack_config_path = if let Some(webpack_config) = &target.webpack_config { - // require webpack_config in wrangler.toml to use it in sites - Some(PathBuf::from(&webpack_config)) - } else if target.site.is_none() { - let config_path = PathBuf::from("webpack.config.js".to_string()); - // backwards compatibility, deprecated in 1.6.0 - // if webpack.config.js exists and is not specified in wrangler.toml, use it and warn - if bundle.has_webpack_config(&config_path) { - message::warn("In Wrangler v1.6.0, you will need to include a webpack_config field in your wrangler.toml to build with a custom webpack configuration."); - Some(config_path) - } else { - // if webpack.config.js does not exist, don't warn, use our default + let custom_webpack_config_path = match &target.webpack_config { + Some(webpack_config) => match &target.site { + None => Some(PathBuf::from(&webpack_config)), + Some(_) => { + message::warn("Workers Sites does not support custom webpack configuration files"); + None + } + }, + None => { + if target.site.is_none() { + let config_path = PathBuf::from("webpack.config.js".to_string()); + if config_path.exists() { + message::warn("If you would like to use your own custom webpack configuration, you will need to add this to your wrangler.toml:\nwebpack_config = \"webpack.config.js\""); + } + } None } - } else { - // don't use `webpack.config.js` if this project is a site - None }; - // if {webpack.config.js} is not present, we infer the entry based on the - // {package.json} file and pass it to {wranglerjs}. - // https://github.com/cloudflare/wrangler/issues/98 - if let Some(webpack_config_path) = webpack_config_path { + // if webpack_config is not configured in the manifest + // we infer the entry based on {package.json} and pass it to {wranglerjs} + if let Some(webpack_config_path) = custom_webpack_config_path { build_with_custom_webpack(&mut command, &webpack_config_path); } else { build_with_default_webpack(&mut command, &build_dir)?; From 0bd86ba082a5b24d5db95a898db6461a2789d9c6 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 5 Nov 2019 15:27:55 -0600 Subject: [PATCH 14/18] Update webpack tests --- tests/build.rs | 62 ++++++------------- .../webpack_target_node/webpack.config.js | 4 ++ .../webpack_target_web/webpack.config.js | 4 ++ .../webpack.config.js | 4 ++ tests/preview.rs | 1 + tests/utils/mod.rs | 15 ----- 6 files changed, 31 insertions(+), 59 deletions(-) create mode 100644 tests/fixtures/webpack_target_node/webpack.config.js create mode 100644 tests/fixtures/webpack_target_web/webpack.config.js create mode 100644 tests/fixtures/webpack_target_webworker/webpack.config.js diff --git a/tests/build.rs b/tests/build.rs index 5240e2dfc..f0d70d998 100644 --- a/tests/build.rs +++ b/tests/build.rs @@ -38,7 +38,8 @@ fn it_builds_with_webpack_single_js() { let fixture = "webpack_simple_js"; utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" + webpack_config = "webpack.config.js" "#}; build(fixture); @@ -52,7 +53,8 @@ fn it_builds_with_webpack_function_config_js() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" + webpack_config = "webpack.config.js" "#}; build(fixture); @@ -66,7 +68,8 @@ fn it_builds_with_webpack_promise_config_js() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" + webpack_config = "webpack.config.js" "#}; build(fixture); @@ -80,7 +83,8 @@ fn it_builds_with_webpack_function_promise_config_js() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" + webpack_config = "webpack.config.js" "#}; build(fixture); @@ -94,7 +98,7 @@ fn it_builds_with_webpack_single_js_use_package_main() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" "#}; build(fixture); @@ -108,7 +112,7 @@ fn it_builds_with_webpack_specify_configs() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" webpack_config = "webpack.worker.js" "#}; @@ -123,7 +127,7 @@ fn it_builds_with_webpack_single_js_missing_package_main() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" "#}; build_fails_with( @@ -139,21 +143,8 @@ fn it_fails_with_multiple_webpack_configs() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" - "#}; - - build_fails_with(fixture, "Multiple webpack configurations are not supported. You can specify a different path for your webpack configuration file in wrangler.toml with the `webpack_config` field"); - utils::cleanup(fixture); -} - -#[test] -fn it_fails_with_multiple_specify_webpack_configs() { - let fixture = "webpack_multiple_specify_config"; - utils::create_temporary_copy(fixture); - - single_env_settings! {fixture, r#" - type = "Webpack" - webpack_config = "webpack.worker.js" + type = "webpack" + webpack_config = "webpack.config.js" "#}; build_fails_with(fixture, "Multiple webpack configurations are not supported. You can specify a different path for your webpack configuration file in wrangler.toml with the `webpack_config` field"); @@ -166,7 +157,8 @@ fn it_builds_with_webpack_wast() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" - type = "Webpack" + type = "webpack" + webpack_config = "webpack.config.js" "#}; build(fixture); @@ -183,15 +175,9 @@ fn it_fails_with_webpack_target_node() { let fixture = "webpack_target_node"; utils::create_temporary_copy(fixture); - utils::webpack_config( - fixture, - r#"{ - entry: "./index.js", - target: "node", - }"#, - ); single_env_settings! {fixture, r#" type = "webpack" + webpack_config = "webpack.config.js" "#}; build_fails_with( @@ -206,15 +192,9 @@ fn it_fails_with_webpack_target_web() { let fixture = "webpack_target_web"; utils::create_temporary_copy(fixture); - utils::webpack_config( - fixture, - r#"{ - entry: "./index.js", - target: "web", - }"#, - ); single_env_settings! {fixture, r#" type = "webpack" + webpack_config = "webpack.config.js" "#}; build_fails_with( @@ -229,15 +209,9 @@ fn it_builds_with_webpack_target_webworker() { let fixture = "webpack_target_webworker"; utils::create_temporary_copy(fixture); - utils::webpack_config( - fixture, - r#"{ - entry: "./index.js", - target: "webworker", - }"#, - ); single_env_settings! {fixture, r#" type = "webpack" + webpack_config = "webpack.config.js" "#}; build(fixture); diff --git a/tests/fixtures/webpack_target_node/webpack.config.js b/tests/fixtures/webpack_target_node/webpack.config.js new file mode 100644 index 000000000..76cbf42d8 --- /dev/null +++ b/tests/fixtures/webpack_target_node/webpack.config.js @@ -0,0 +1,4 @@ +module.exports = { + entry: "./index.js", + target: "node", +} \ No newline at end of file diff --git a/tests/fixtures/webpack_target_web/webpack.config.js b/tests/fixtures/webpack_target_web/webpack.config.js new file mode 100644 index 000000000..02b18bdb1 --- /dev/null +++ b/tests/fixtures/webpack_target_web/webpack.config.js @@ -0,0 +1,4 @@ +module.exports = { + entry: "./index.js", + target: "web", +} \ No newline at end of file diff --git a/tests/fixtures/webpack_target_webworker/webpack.config.js b/tests/fixtures/webpack_target_webworker/webpack.config.js new file mode 100644 index 000000000..374b67d3b --- /dev/null +++ b/tests/fixtures/webpack_target_webworker/webpack.config.js @@ -0,0 +1,4 @@ +module.exports = { + entry: "./index.js", + target: "webworker", +} \ No newline at end of file diff --git a/tests/preview.rs b/tests/preview.rs index 17bcd4d05..8fba503b5 100644 --- a/tests/preview.rs +++ b/tests/preview.rs @@ -50,6 +50,7 @@ fn it_can_preview_webpack_project() { utils::create_temporary_copy(fixture); settings! {fixture, r#" type = "webpack" + webpack_config = "webpack.config.js" "#}; preview(fixture); utils::cleanup(fixture); diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs index 457632885..fe48568ba 100644 --- a/tests/utils/mod.rs +++ b/tests/utils/mod.rs @@ -1,8 +1,6 @@ use fs_extra::dir::{copy, CopyOptions}; use std::env; use std::fs; -use std::fs::File; -use std::io::prelude::*; use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::Mutex; @@ -53,16 +51,3 @@ pub fn create_temporary_copy(fixture: &str) { options.overwrite = true; copy(src, dest, &options).unwrap(); } - -// TODO: remove once https://github.com/cloudflare/wrangler/pull/489 is merged -pub fn webpack_config(fixture: &str, config: &str) { - let file_path = fixture_path(fixture).join("webpack.config.js"); - let mut file = File::create(file_path).unwrap(); - let content = format!( - r#" - module.exports = {}; - "#, - config - ); - file.write_all(content.as_bytes()).unwrap(); -} From 6cba9be78a5a40ec9b30298de1fd9c703c74ffc8 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Wed, 6 Nov 2019 14:04:27 -0600 Subject: [PATCH 15/18] Fix webpack tests --- tests/build.rs | 69 ++++++++++++++++++- .../package.json | 4 +- .../webpack.config.js | 1 - .../webpack_multiple_config/package.json | 4 +- .../webpack_multiple_config/webpack.config.js | 4 -- .../webpack_multiple_specify_config/index.js | 0 .../package.json | 1 - .../webpack.worker.js | 4 -- tests/fixtures/webpack_simple_js/package.json | 4 +- .../webpack_simple_js/webpack.config.js | 1 - .../webpack_specify_config/package.json | 4 +- .../webpack_specify_config/webpack.worker.js | 1 - .../fixtures/webpack_target_node/package.json | 3 + .../webpack_target_node/webpack.config.js | 4 -- .../fixtures/webpack_target_web/package.json | 3 + .../webpack_target_web/webpack.config.js | 4 -- .../webpack_target_webworker/package.json | 3 + .../webpack.config.js | 4 -- tests/utils/mod.rs | 10 +++ 19 files changed, 99 insertions(+), 29 deletions(-) delete mode 100644 tests/fixtures/webpack_function_promise_config_js/webpack.config.js delete mode 100644 tests/fixtures/webpack_multiple_config/webpack.config.js delete mode 100644 tests/fixtures/webpack_multiple_specify_config/index.js delete mode 100644 tests/fixtures/webpack_multiple_specify_config/package.json delete mode 100644 tests/fixtures/webpack_multiple_specify_config/webpack.worker.js delete mode 100644 tests/fixtures/webpack_simple_js/webpack.config.js delete mode 100644 tests/fixtures/webpack_specify_config/webpack.worker.js create mode 100644 tests/fixtures/webpack_target_node/package.json delete mode 100644 tests/fixtures/webpack_target_node/webpack.config.js create mode 100644 tests/fixtures/webpack_target_web/package.json delete mode 100644 tests/fixtures/webpack_target_web/webpack.config.js create mode 100644 tests/fixtures/webpack_target_webworker/package.json delete mode 100644 tests/fixtures/webpack_target_webworker/webpack.config.js diff --git a/tests/build.rs b/tests/build.rs index f0d70d998..7f0208823 100644 --- a/tests/build.rs +++ b/tests/build.rs @@ -39,7 +39,6 @@ fn it_builds_with_webpack_single_js() { utils::create_temporary_copy(fixture); single_env_settings! {fixture, r#" type = "webpack" - webpack_config = "webpack.config.js" "#}; build(fixture); @@ -67,6 +66,14 @@ fn it_builds_with_webpack_promise_config_js() { let fixture = "webpack_promise_config_js"; utils::create_temporary_copy(fixture); + utils::create_fixture_file( + fixture, + "webpack.config.js", + r#" + module.exports = Promise.resolve({ entry: "./index.js" }); + "#, + ); + single_env_settings! {fixture, r#" type = "webpack" webpack_config = "webpack.config.js" @@ -82,6 +89,14 @@ fn it_builds_with_webpack_function_promise_config_js() { let fixture = "webpack_function_promise_config_js"; utils::create_temporary_copy(fixture); + utils::create_fixture_file( + fixture, + "webpack.config.js", + r#" + module.exports = Promise.resolve({ entry: "./index.js" }); + "#, + ); + single_env_settings! {fixture, r#" type = "webpack" webpack_config = "webpack.config.js" @@ -111,6 +126,14 @@ fn it_builds_with_webpack_specify_configs() { let fixture = "webpack_specify_config"; utils::create_temporary_copy(fixture); + utils::create_fixture_file( + fixture, + "webpack.worker.js", + r#" + module.exports = { entry: "./index.js" }; + "#, + ); + single_env_settings! {fixture, r#" type = "webpack" webpack_config = "webpack.worker.js" @@ -142,6 +165,17 @@ fn it_fails_with_multiple_webpack_configs() { let fixture = "webpack_multiple_config"; utils::create_temporary_copy(fixture); + utils::create_fixture_file( + fixture, + "webpack.config.js", + r#" + module.exports = [ + { entry: "./a.js" }, + { entry: "./b.js" } + ] + "#, + ); + single_env_settings! {fixture, r#" type = "webpack" webpack_config = "webpack.config.js" @@ -175,6 +209,17 @@ fn it_fails_with_webpack_target_node() { let fixture = "webpack_target_node"; utils::create_temporary_copy(fixture); + utils::create_fixture_file( + fixture, + "webpack.config.js", + r#" + module.exports = { + "entry": "./index.js", + "target": "node" + } + "#, + ); + single_env_settings! {fixture, r#" type = "webpack" webpack_config = "webpack.config.js" @@ -192,6 +237,17 @@ fn it_fails_with_webpack_target_web() { let fixture = "webpack_target_web"; utils::create_temporary_copy(fixture); + utils::create_fixture_file( + fixture, + "webpack.config.js", + r#" + module.exports = { + "entry": "./index.js", + "target": "web" + } + "#, + ); + single_env_settings! {fixture, r#" type = "webpack" webpack_config = "webpack.config.js" @@ -209,6 +265,17 @@ fn it_builds_with_webpack_target_webworker() { let fixture = "webpack_target_webworker"; utils::create_temporary_copy(fixture); + utils::create_fixture_file( + fixture, + "webpack.config.js", + r#" + module.exports = { + "entry": "./index.js", + "target": "webworker" + } + "#, + ); + single_env_settings! {fixture, r#" type = "webpack" webpack_config = "webpack.config.js" diff --git a/tests/fixtures/webpack_function_promise_config_js/package.json b/tests/fixtures/webpack_function_promise_config_js/package.json index 0967ef424..f68471d54 100644 --- a/tests/fixtures/webpack_function_promise_config_js/package.json +++ b/tests/fixtures/webpack_function_promise_config_js/package.json @@ -1 +1,3 @@ -{} +{ + "main": "index.js" +} diff --git a/tests/fixtures/webpack_function_promise_config_js/webpack.config.js b/tests/fixtures/webpack_function_promise_config_js/webpack.config.js deleted file mode 100644 index 545658983..000000000 --- a/tests/fixtures/webpack_function_promise_config_js/webpack.config.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = () => Promise.resolve({ entry: "./index.js" }); diff --git a/tests/fixtures/webpack_multiple_config/package.json b/tests/fixtures/webpack_multiple_config/package.json index 0967ef424..f68471d54 100644 --- a/tests/fixtures/webpack_multiple_config/package.json +++ b/tests/fixtures/webpack_multiple_config/package.json @@ -1 +1,3 @@ -{} +{ + "main": "index.js" +} diff --git a/tests/fixtures/webpack_multiple_config/webpack.config.js b/tests/fixtures/webpack_multiple_config/webpack.config.js deleted file mode 100644 index faf40ebac..000000000 --- a/tests/fixtures/webpack_multiple_config/webpack.config.js +++ /dev/null @@ -1,4 +0,0 @@ -module.exports = [ - { entry: "./a.js" }, - { entry: "./b.js" } -] diff --git a/tests/fixtures/webpack_multiple_specify_config/index.js b/tests/fixtures/webpack_multiple_specify_config/index.js deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/fixtures/webpack_multiple_specify_config/package.json b/tests/fixtures/webpack_multiple_specify_config/package.json deleted file mode 100644 index 9e26dfeeb..000000000 --- a/tests/fixtures/webpack_multiple_specify_config/package.json +++ /dev/null @@ -1 +0,0 @@ -{} \ No newline at end of file diff --git a/tests/fixtures/webpack_multiple_specify_config/webpack.worker.js b/tests/fixtures/webpack_multiple_specify_config/webpack.worker.js deleted file mode 100644 index fb177715e..000000000 --- a/tests/fixtures/webpack_multiple_specify_config/webpack.worker.js +++ /dev/null @@ -1,4 +0,0 @@ -module.exports = [ - { entry: "./a.js" }, - { entry: "./b.js" } - ] \ No newline at end of file diff --git a/tests/fixtures/webpack_simple_js/package.json b/tests/fixtures/webpack_simple_js/package.json index 0967ef424..f68471d54 100644 --- a/tests/fixtures/webpack_simple_js/package.json +++ b/tests/fixtures/webpack_simple_js/package.json @@ -1 +1,3 @@ -{} +{ + "main": "index.js" +} diff --git a/tests/fixtures/webpack_simple_js/webpack.config.js b/tests/fixtures/webpack_simple_js/webpack.config.js deleted file mode 100644 index f07507ec6..000000000 --- a/tests/fixtures/webpack_simple_js/webpack.config.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = { entry: "./index.js" }; diff --git a/tests/fixtures/webpack_specify_config/package.json b/tests/fixtures/webpack_specify_config/package.json index 0967ef424..f68471d54 100644 --- a/tests/fixtures/webpack_specify_config/package.json +++ b/tests/fixtures/webpack_specify_config/package.json @@ -1 +1,3 @@ -{} +{ + "main": "index.js" +} diff --git a/tests/fixtures/webpack_specify_config/webpack.worker.js b/tests/fixtures/webpack_specify_config/webpack.worker.js deleted file mode 100644 index f07507ec6..000000000 --- a/tests/fixtures/webpack_specify_config/webpack.worker.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = { entry: "./index.js" }; diff --git a/tests/fixtures/webpack_target_node/package.json b/tests/fixtures/webpack_target_node/package.json new file mode 100644 index 000000000..f68471d54 --- /dev/null +++ b/tests/fixtures/webpack_target_node/package.json @@ -0,0 +1,3 @@ +{ + "main": "index.js" +} diff --git a/tests/fixtures/webpack_target_node/webpack.config.js b/tests/fixtures/webpack_target_node/webpack.config.js deleted file mode 100644 index 76cbf42d8..000000000 --- a/tests/fixtures/webpack_target_node/webpack.config.js +++ /dev/null @@ -1,4 +0,0 @@ -module.exports = { - entry: "./index.js", - target: "node", -} \ No newline at end of file diff --git a/tests/fixtures/webpack_target_web/package.json b/tests/fixtures/webpack_target_web/package.json new file mode 100644 index 000000000..f68471d54 --- /dev/null +++ b/tests/fixtures/webpack_target_web/package.json @@ -0,0 +1,3 @@ +{ + "main": "index.js" +} diff --git a/tests/fixtures/webpack_target_web/webpack.config.js b/tests/fixtures/webpack_target_web/webpack.config.js deleted file mode 100644 index 02b18bdb1..000000000 --- a/tests/fixtures/webpack_target_web/webpack.config.js +++ /dev/null @@ -1,4 +0,0 @@ -module.exports = { - entry: "./index.js", - target: "web", -} \ No newline at end of file diff --git a/tests/fixtures/webpack_target_webworker/package.json b/tests/fixtures/webpack_target_webworker/package.json new file mode 100644 index 000000000..f68471d54 --- /dev/null +++ b/tests/fixtures/webpack_target_webworker/package.json @@ -0,0 +1,3 @@ +{ + "main": "index.js" +} diff --git a/tests/fixtures/webpack_target_webworker/webpack.config.js b/tests/fixtures/webpack_target_webworker/webpack.config.js deleted file mode 100644 index 374b67d3b..000000000 --- a/tests/fixtures/webpack_target_webworker/webpack.config.js +++ /dev/null @@ -1,4 +0,0 @@ -module.exports = { - entry: "./index.js", - target: "webworker", -} \ No newline at end of file diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs index fe48568ba..2a7c1d0b5 100644 --- a/tests/utils/mod.rs +++ b/tests/utils/mod.rs @@ -1,6 +1,8 @@ use fs_extra::dir::{copy, CopyOptions}; use std::env; use std::fs; +use std::fs::File; +use std::io::prelude::*; use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::Mutex; @@ -51,3 +53,11 @@ pub fn create_temporary_copy(fixture: &str) { options.overwrite = true; copy(src, dest, &options).unwrap(); } + +pub fn create_fixture_file(fixture: &str, name: &str, content: &str) { + let file_path = fixture_path(fixture).join(name); + println!("{:?}", file_path); + let mut file = File::create(file_path).unwrap(); + let content = String::from(content); + file.write_all(content.as_bytes()).unwrap(); +} From 4c8a1d9ea4404949fb122551b231a0689b4ffd60 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Wed, 6 Nov 2019 14:34:05 -0600 Subject: [PATCH 16/18] Make preview test pass --- tests/preview.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/preview.rs b/tests/preview.rs index 8fba503b5..79e0dda57 100644 --- a/tests/preview.rs +++ b/tests/preview.rs @@ -5,6 +5,7 @@ pub mod utils; use assert_cmd::prelude::*; +use std::env; use std::fs::File; use std::io::Write; use std::process::Command; @@ -50,7 +51,6 @@ fn it_can_preview_webpack_project() { utils::create_temporary_copy(fixture); settings! {fixture, r#" type = "webpack" - webpack_config = "webpack.config.js" "#}; preview(fixture); utils::cleanup(fixture); @@ -70,7 +70,7 @@ fn it_can_preview_rust_project() { fn preview(fixture: &str) { // Lock to avoid having concurrent builds let _g = BUILD_LOCK.lock().unwrap(); - + env::remove_var("CF_ACCOUNT_ID"); let mut preview = Command::cargo_bin(env!("CARGO_PKG_NAME")).unwrap(); preview.current_dir(utils::fixture_path(fixture)); preview.arg("preview").arg("--headless").assert().success(); From afdd02318c254ec5b4671ac9e8678c425eb5a115 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Wed, 6 Nov 2019 16:28:57 -0600 Subject: [PATCH 17/18] Clean up wranglerjs temp file --- src/commands/build/wranglerjs/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index 12f58dd7c..756e64f1c 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -43,8 +43,8 @@ pub fn run_build(target: &Target) -> Result<(), failure::Error> { let status = command.status()?; if status.success() { - let output = fs::read_to_string(temp_file).expect("could not retrieve output"); - + let output = fs::read_to_string(&temp_file).expect("could not retrieve output"); + fs::remove_file(temp_file)?; let wranglerjs_output: WranglerjsOutput = serde_json::from_str(&output).expect("could not parse wranglerjs output"); From b6e636020259632aea32e8177f8e4e83865d5660 Mon Sep 17 00:00:00 2001 From: Gabbi Fisher Date: Fri, 8 Nov 2019 08:35:34 -0800 Subject: [PATCH 18/18] Refactor cloudflare-rs cli to be called from http.rs (#841) * Refactor cloudflare-rs cli to call from http.rs * pass functions into format_error for help annotations * s/todo/TODO * get rid of unused enum * get rid of empty string logic in favor of Some/None --- src/commands/kv/mod.rs | 155 ++++++++++++++++------------------------- src/http.rs | 67 ++++++++++++++++++ 2 files changed, 128 insertions(+), 94 deletions(-) diff --git a/src/commands/kv/mod.rs b/src/commands/kv/mod.rs index 419466287..932a8a581 100644 --- a/src/commands/kv/mod.rs +++ b/src/commands/kv/mod.rs @@ -1,17 +1,15 @@ use std::collections::HashSet; use std::time::Duration; -use cloudflare::framework::auth::Credentials; use cloudflare::framework::response::ApiFailure; -use cloudflare::framework::{Environment, HttpApiClient, HttpApiClientConfig}; +use cloudflare::framework::{HttpApiClient, HttpApiClientConfig}; -use http::status::StatusCode; use percent_encoding::{percent_encode, PATH_SEGMENT_ENCODE_SET}; use crate::settings::global_user::GlobalUser; use crate::settings::target::Target; -use crate::terminal::emoji; -use crate::terminal::message; + +use crate::http; pub mod bucket; pub mod bulk; @@ -23,6 +21,64 @@ const INTERACTIVE_RESPONSE_LEN: usize = 1; const YES: &str = "y"; const NO: &str = "n"; +// Create a special API client that has a longer timeout than usual, given that KV operations +// can be lengthy if payloads are large. +fn api_client(user: &GlobalUser) -> Result { + http::api_client( + user, + HttpApiClientConfig { + // Use 5 minute timeout instead of default 30-second one. + // This is useful for bulk upload operations. + http_timeout: Duration::from_secs(5 * 60), + }, + ) +} + +fn format_error(e: ApiFailure) -> String { + http::format_error(e, Some(&kv_help)) +} + +// kv_help() provides more detailed explanations of Workers KV API error codes. +// See https://api.cloudflare.com/#workers-kv-namespace-errors for details. +fn kv_help(error_code: u16) -> &'static str { + match error_code { + 7003 | 7000 => { + "Your wrangler.toml is likely missing the field \"account_id\", which is required to write to Workers KV." + } + // namespace errors + 10010 | 10011 | 10012 | 10013 | 10014 | 10018 => { + "Run `wrangler kv:namespace list` to see your existing namespaces with IDs" + } + 10009 => "Run `wrangler kv:key list` to see your existing keys", // key errors + // TODO: link to more info + // limit errors + 10022 | 10024 | 10030 => "See documentation", + // TODO: link to tool for this? + // legacy namespace errors + 10021 | 10035 | 10038 => "Consider moving this namespace", + // cloudflare account errors + 10017 | 10026 => "Workers KV is a paid feature, please upgrade your account (https://www.cloudflare.com/products/workers-kv/)", + _ => "", + } +} + +pub fn validate_target(target: &Target) -> Result<(), failure::Error> { + let mut missing_fields = Vec::new(); + + if target.account_id.is_empty() { + missing_fields.push("account_id") + }; + + if !missing_fields.is_empty() { + failure::bail!( + "Your wrangler.toml is missing the following field(s): {:?}", + missing_fields + ) + } else { + Ok(()) + } +} + fn check_duplicate_namespaces(target: &Target) -> bool { // HashSet for detecting duplicate namespace bindings let mut binding_names: HashSet = HashSet::new(); @@ -64,41 +120,6 @@ pub fn get_namespace_id(target: &Target, binding: &str) -> Result Result { - HttpApiClient::new( - Credentials::from(user.to_owned()), - HttpApiClientConfig { - // Use 5 minute timeout instead of default 30-second one. - // This is useful for bulk upload operations. - http_timeout: Duration::from_secs(5 * 60), - }, - Environment::Production, - ) -} - -fn format_error(e: ApiFailure) -> String { - match e { - ApiFailure::Error(status, api_errors) => { - print_status_code_context(status); - let mut complete_err = "".to_string(); - for error in api_errors.errors { - let error_msg = - format!("{} Error {}: {}\n", emoji::WARN, error.code, error.message); - - let suggestion = help(error.code); - if !suggestion.is_empty() { - let help_msg = format!("{} {}\n", emoji::SLEUTH, suggestion); - complete_err.push_str(&format!("{}{}", error_msg, help_msg)); - } else { - complete_err.push_str(&error_msg) - } - } - complete_err - } - ApiFailure::Invalid(reqwest_err) => format!("{} Error: {}", emoji::WARN, reqwest_err), - } -} - // For interactively handling deletes (and discouraging accidental deletes). // Input like "yes", "Yes", "no", "No" will be accepted, thanks to the whitespace-stripping // and lowercasing logic below. @@ -119,60 +140,6 @@ fn url_encode_key(key: &str) -> String { percent_encode(key.as_bytes(), PATH_SEGMENT_ENCODE_SET).to_string() } -// For handling cases where the API gateway returns errors via HTTP status codes -// (no KV error code is given). -fn print_status_code_context(status_code: StatusCode) { - match status_code { - // Folks should never hit PAYLOAD_TOO_LARGE, given that Wrangler ensures that bulk file uploads - // are max ~50 MB in size. This case is handled anyways out of an abundance of caution. - StatusCode::PAYLOAD_TOO_LARGE => - message::warn("Returned status code 413, Payload Too Large. Please make sure your upload is less than 100MB in size"), - StatusCode::GATEWAY_TIMEOUT => - message::warn("Returned status code 504, Gateway Timeout. Please try again in a few seconds"), - _ => (), - } -} - -fn help(error_code: u16) -> &'static str { - // https://api.cloudflare.com/#workers-kv-namespace-errors - match error_code { - 7003 | 7000 => { - "Your wrangler.toml is likely missing the field \"account_id\", which is required to write to Workers KV." - } - // namespace errors - 10010 | 10011 | 10012 | 10013 | 10014 | 10018 => { - "Run `wrangler kv:namespace list` to see your existing namespaces with IDs" - } - 10009 => "Run `wrangler kv:key list` to see your existing keys", // key errors - // TODO: link to more info - // limit errors - 10022 | 10024 | 10030 => "See documentation", - // TODO: link to tool for this? - // legacy namespace errors - 10021 | 10035 | 10038 => "Consider moving this namespace", - // cloudflare account errors - 10017 | 10026 => "Workers KV is a paid feature, please upgrade your account (https://www.cloudflare.com/products/workers-kv/)", - _ => "", - } -} - -pub fn validate_target(target: &Target) -> Result<(), failure::Error> { - let mut missing_fields = Vec::new(); - - if target.account_id.is_empty() { - missing_fields.push("account_id") - }; - - if !missing_fields.is_empty() { - failure::bail!( - "Your wrangler.toml is missing the following field(s): {:?}", - missing_fields - ) - } else { - Ok(()) - } -} - #[cfg(test)] mod tests { use crate::commands::kv; diff --git a/src/http.rs b/src/http.rs index 876d257f2..2c2a9fcac 100644 --- a/src/http.rs +++ b/src/http.rs @@ -5,6 +5,17 @@ use std::time::Duration; use crate::install; use crate::settings::global_user::GlobalUser; +use http::status::StatusCode; + +use cloudflare::framework::auth::Credentials; +use cloudflare::framework::response::ApiFailure; +use cloudflare::framework::{Environment, HttpApiClient, HttpApiClientConfig}; + +use crate::terminal::emoji; +use crate::terminal::message; + +////---------------------------OLD API CLIENT CODE---------------------------//// +// TODO: remove this and replace it entirely with cloudflare-rs fn headers(feature: Option<&str>) -> HeaderMap { let version = if install::target::DEBUG { "dev" @@ -61,3 +72,59 @@ fn add_auth_headers<'a>(headers: &'a mut HeaderMap, user: &GlobalUser) { } } } + +////---------------------------NEW API CLIENT CODE---------------------------//// +pub fn api_client( + user: &GlobalUser, + config: HttpApiClientConfig, +) -> Result { + HttpApiClient::new( + Credentials::from(user.to_owned()), + config, + Environment::Production, + ) +} + +// Format errors from the cloudflare-rs cli for printing. +// Optionally takes an argument for providing a function that maps error code numbers to +// helpful additional information about why someone is getting an error message and how to fix it. +pub fn format_error(e: ApiFailure, err_helper: Option<&dyn Fn(u16) -> &'static str>) -> String { + match e { + ApiFailure::Error(status, api_errors) => { + print_status_code_context(status); + let mut complete_err = "".to_string(); + for error in api_errors.errors { + let error_msg = + format!("{} Error {}: {}\n", emoji::WARN, error.code, error.message); + + let suggestion = if let Some(annotate_help) = err_helper { + Some(annotate_help(error.code)) + } else { + None + }; + if let Some(suggestion_text) = suggestion { + let help_msg = format!("{} {}\n", emoji::SLEUTH, suggestion_text); + complete_err.push_str(&format!("{}{}", error_msg, help_msg)); + } else { + complete_err.push_str(&error_msg) + } + } + complete_err + } + ApiFailure::Invalid(reqwest_err) => format!("{} Error: {}", emoji::WARN, reqwest_err), + } +} + +// For handling cases where the API gateway returns errors via HTTP status codes +// (no API-specific, more granular error code is given). +fn print_status_code_context(status_code: StatusCode) { + match status_code { + // Folks should never hit PAYLOAD_TOO_LARGE, given that Wrangler ensures that bulk file uploads + // are max ~50 MB in size. This case is handled anyways out of an abundance of caution. + StatusCode::PAYLOAD_TOO_LARGE => + message::warn("Returned status code 413, Payload Too Large. Please make sure your upload is less than 100MB in size"), + StatusCode::GATEWAY_TIMEOUT => + message::warn("Returned status code 504, Gateway Timeout. Please try again in a few seconds"), + _ => (), + } +}