Merge branch 'master' into avery/route-66

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
 

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;

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<mpsc::Sender<()>>,

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<DebouncedEvent>,
     cooldown: Duration,

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()
     }

src/commands/build/wranglerjs/mod.rs

@@ -30,9 +30,9 @@ 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
-// executable and wait for completion. The file will receive the a serialized
+
+// In Rust we create a virtual file, pass it to {wranglerjs}, run the
+// 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> {
@@ -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");
 
@@ -62,7 +62,7 @@ pub fn run_build_and_watch(target: &Target, tx: Option<Sender<()>>) -> 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<Sender<()>>) -> 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);
 
@@ -175,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)?;
@@ -230,7 +230,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 +267,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)?;
     }

src/commands/build/wranglerjs/output.rs

@@ -6,8 +6,8 @@ use serde::Deserialize;
 use std::io::prelude::*;
 
 // This structure represents the communication between {wranglerjs} and
-// {wrangler}. It is send back after {wranglerjs} completion.
-// FIXME(sven): make this private
+// {wrangler}. It is sent back after {wranglerjs} completion.
+// TODO: (sven) make this private
 #[derive(Deserialize, Debug)]
 pub struct WranglerjsOutput {
     pub wasm: Option<String>,
@@ -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);

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)

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;

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).

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).