Add some documentation

src/build/check/config.rs

@@ -6,7 +6,7 @@ use wasmparser::WasmFeatures;
 /// Check the [SWC docs](https://swc.rs/rustdoc/swc_ecma_parser/struct.EsConfig.html) for the available options.
 #[doc(inline)]
 pub const V8_SUPPORTED_JS_FEATURES: Syntax = Syntax::Es(EsConfig {
-    // sir, this is a wendy's...
+    // sir, this is a wendy's...(we don't allow JSX because V8 doesn't parse JSX)
     jsx: false,
     // https://v8.dev/blog/v8-release-75#numeric-separators
     num_sep: true,
@@ -18,7 +18,7 @@ pub const V8_SUPPORTED_JS_FEATURES: Syntax = Syntax::Es(EsConfig {
     class_props: true,
     // https://chromium.googlesource.com/v8/v8/+/3.0.12.1/test/mjsunit/function-bind.js
     fn_bind: true,
-    // AFAIK this is still...due to be presented? it is now october but
+    // AFAIK this is still...due to be presented? it is now ~~september october~~ november but
     // applies to both decorators and decorators_before_export
     // rfc: https://github.com/tc39/proposal-decorators
     // V8 team's feedback: https://docs.google.com/document/d/1GMp938qlmJlGkBZp6AerL-ewL1MWUDU8QzHBiNvs3MM/edit
@@ -49,7 +49,7 @@ pub const V8_SUPPORTED_JS_FEATURES: Syntax = Syntax::Es(EsConfig {
 
 /// The features we allow during our validation of WebAssembly, as per V8 stable.
 /// Check the [wasmparser](https://docs.rs/wasmparser/0.63.0/wasmparser/struct.WasmFeatures.html)
-/// for more.
+/// docs for more info.
 #[doc(inline)]
 pub const V8_SUPPORTED_WASM_FEATURES: WasmFeatures = WasmFeatures {
     // https://www.chromestatus.com/feature/5166497248837632
@@ -60,8 +60,6 @@ pub const V8_SUPPORTED_WASM_FEATURES: WasmFeatures = WasmFeatures {
     // https://v8.dev/blog/webassembly-experimental
     // so i'm not sure what the right answer for this is.
     // either way, i wasn't able to find a chromestatus page for it.
-    // based on the name, though, i don't think it matters
-    // because Workers only supports one WASM file anyway
     module_linking: false,
     // https://www.chromestatus.com/feature/6533147810332672
     simd: false,

src/build/check/js/lint.rs

@@ -55,7 +55,20 @@ impl JavaScriptLinter {
     }
 }
 
+/// TODO: this still needs to be done
+/// basically the way this works is that by implementing `Visit` on `JavaScriptLinter`,
+/// we can then visit every node in SWC's AST with an instance of JavaScriptLinter.
+/// So what we need to do is pick the nodes we want to lint, and then implement their
+/// respective trait functions to check them and add errors via `JavaScriptLinter::error()`.
+///
+/// So, for example, when visiting a ModuleDecl, we might check to make sure that if it's
+/// and export, that export is either a Class (meaning its a durable object) or a default
+/// export that has a `fetch` handler, or something like that.
+/// This is essentially the actual bulk of the work, the rest has just been plumbing.
+///
+/// It's been a lot of plumbing, though :sweat_smile:
 impl Visit for JavaScriptLinter {
+    // this is an incomplete example of what the linter will look like
     fn visit_module_decl(&mut self, n: &ModuleDecl, _parent: &dyn Node) {
         match n {
             ModuleDecl::Import(import) => {

src/build/check/js/mod.rs

@@ -5,13 +5,14 @@ use swc_common::{sync::Lrc, SourceMap as SwcSourceMap};
 use swc_ecma_ast::{ImportDecl, Module};
 use swc_ecma_visit::{Node, Visit, VisitWith};
 
-use super::{Lintable, Parseable, Validate};
+use super::{config::V8_SUPPORTED_JS_FEATURES, Lintable, Parseable, Validate};
 
-pub mod lint;
-pub mod parse;
-
-use super::config::V8_SUPPORTED_JS_FEATURES;
+// bring implemntations of Lintable and Parseable into scope
+mod lint;
+mod parse;
 
+/// A representation of a JS file (+ an optional source map)
+/// produced by a bundler's output
 pub struct JavaScript {
     module: Module,
     swc_source_map: Lrc<SwcSourceMap>,
@@ -31,13 +32,15 @@ impl std::fmt::Debug for JavaScript {
 }
 
 impl JavaScript {
+    /// Find any other files imported by this JS file
     pub fn find_imports(&self) -> Vec<String> {
         let mut import_finder = ImportFinder { imports: vec![] };
         self.module.visit_children_with(&mut import_finder);
         import_finder.imports
     }
 }
 
+#[doc = "hidden"]
 struct ImportFinder {
     pub imports: Vec<String>,
 }

src/build/check/js/parse.rs

@@ -18,7 +18,7 @@ impl Parseable<PathBuf> for JavaScript {
         // TODO ask in the slack
         let mut parser = Parser::new(V8_SUPPORTED_JS_FEATURES, StringInput::from(&*fm), None);
 
-        // TODO
+        // TODO: need feedback
         // ok so these errors are recoverable, like we can successfully parse it.
         // if we wanted to be stricter, we could just Err here
         // we could also warn the user about these, but i think

src/build/check/mod.rs

@@ -69,8 +69,10 @@ pub trait Lintable {
     fn lint(&self) -> Result<(), failure::Error>;
 }
 
-/// If a struct is both Parseable and Lintable, then a blanket implementation
-/// is provided for that struct to Validate, which simply combines both steps.
+/// If a struct is both Parseable and Lintable, then it may also implement
+/// Validate, which should serve as a one-stop-shop to go from un-parsed
+/// input to linted output. A default implmenetation is provided which
+/// simply calls `Self::parse(&input).lint()`
 /// ```
 /// # trait Parseable<ParserInput>: Sized {
 /// #    fn parse(input: &ParserInput) -> Result<Self, failure::Error>;
@@ -123,15 +125,16 @@ pub trait Validate<ParserInput>: Lintable + Parseable<ParserInput> {
 ///
 /// I'm not 100% on the format of this struct because I don't
 /// have access to the durable objects beta
-/// but it seems as though the format is basically one .mjs file
-/// that serves as the entrypoint to the worker,
-/// and any number other arbitrary files that can be imported into
-/// the worker. The example on GitHub, for example, imports HTML,
+/// but it seems as though the format is basically:
+/// * one `.mjs` file that serves as the entrypoint to the worker,
+/// * any number other arbitrary files that can be imported into the worker.
+///
+/// The example on GitHub, for example, imports HTML,
 /// so I think that's fair to assume.
 ///
 /// The ones we execute server-side are JS and WebAssembly, so those
 /// get their own `HashMap`s, and any other files can just be assumed
-/// to be static e.g. HTML.
+/// to be static e.g. HTML which means they don't need to be `Validate`d.
 #[derive(Debug)]
 pub struct BundlerOutput {
     /// A PathBuf pointing to worker.mjs, the entrypoint of the worker
@@ -146,8 +149,15 @@ pub struct BundlerOutput {
     other_files: Vec<PathBuf>,
 }
 
-/// Starting by parsing the entrypoint to the worker, traverse the imports
-/// and add those to the bundle as necessary.
+/// Construct an in-memory representation of a bundler's output given
+/// the output dir.
+///
+/// Starting by parsing <output_dir>/worker.mjs, work through its
+/// imports and add those files to the output as necessary.
+///
+/// Notably, any file emitted by the bundler which is not touched by either
+/// worker.mjs or any of its imports (or any of its imports' imports, etc.)
+/// will not be added to the resulting BundlerOutput
 impl<P: AsRef<Path> + Debug> Parseable<P> for BundlerOutput {
     fn parse(output_dir: &P) -> Result<Self, failure::Error> {
         let entry_file = output_dir.as_ref().join(WORKER_FILE_NAME);
@@ -157,8 +167,10 @@ impl<P: AsRef<Path> + Debug> Parseable<P> for BundlerOutput {
         let mut webassembly = HashMap::new();
         let mut other_files = vec![];
 
+        // Create a stack of the imports in the worker entrypoint
         let mut imports = entry.find_imports();
 
+        // Work through the stack, adding more imports as necessary
         while let Some(import) = imports.pop() {
             let import_path = output_dir.as_ref().join(&import);
 
@@ -172,7 +184,7 @@ impl<P: AsRef<Path> + Debug> Parseable<P> for BundlerOutput {
                 Some(extension) => {
                     if let Some(ext_str) = extension.to_str() {
                         match ext_str {
-                            "js" => {
+                            "js" | "mjs" => {
                                 if !javascript.contains_key(&import_path) {
                                     let js_import = JavaScript::parse(&import_path)?;
                                     imports.extend(js_import.find_imports());
@@ -201,6 +213,8 @@ impl<P: AsRef<Path> + Debug> Parseable<P> for BundlerOutput {
                                 }
                             }
                             _ => {
+                                // Since all we execute server-side is javascript and webassembly,
+                                // we can assume these files aren't actually executed.
                                 if !other_files.contains(&import_path) {
                                     other_files.push(import_path);
                                 }
@@ -221,6 +235,10 @@ impl<P: AsRef<Path> + Debug> Parseable<P> for BundlerOutput {
     }
 }
 
+/// Check the sizes of all the files the user wants to upload,
+/// and then lint them all. I suspect this would be a good
+/// use case for rayon, but I'm reluctant to add more dependencies
+/// than are absolutely necessary for this PR
 impl Lintable for BundlerOutput {
     fn lint(&self) -> Result<(), failure::Error> {
         // Check file sizes
@@ -238,4 +256,4 @@ impl Lintable for BundlerOutput {
     }
 }
 
-impl<P: AsRef<Path> + Debug> Validate<P> for BundlerOutput {}
+impl<P: AsRef<Path> + Debug> Validate<P> for BundlerOutput {}

src/build/check/util.rs

@@ -21,41 +21,38 @@ pub fn check_file_size(file: &PathBuf, max_size: ByteSize) -> Result<(), failure
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
 
-    #[cfg(test)]
-    mod check_file_size {
-        use super::super::check_file_size;
-        use bytesize::{ByteSize, MB};
-        use rand::{distributions::Alphanumeric, thread_rng, Rng};
-        use std::{convert::TryInto, io::Write};
+    use super::check_file_size;
+    use bytesize::{ByteSize, MB};
+    use rand::{distributions::Alphanumeric, thread_rng, Rng};
+    use std::{convert::TryInto, io::Write};
 
-        #[test]
-        fn its_ok_with_small_files() -> Result<(), failure::Error> {
-            let file = tempfile::NamedTempFile::new()?;
-            let path_buf = file.path().to_path_buf();
+    #[test]
+    fn its_ok_with_small_files() -> Result<(), failure::Error> {
+        let file = tempfile::NamedTempFile::new()?;
+        let path_buf = file.path().to_path_buf();
 
-            assert!(check_file_size(&path_buf, ByteSize::mb(1)).is_ok());
+        assert!(check_file_size(&path_buf, ByteSize::mb(1)).is_ok());
 
-            Ok(())
-        }
+        Ok(())
+    }
 
-        #[test]
-        fn it_errors_when_file_is_too_big() -> Result<(), failure::Error> {
-            let mut file = tempfile::NamedTempFile::new()?;
+    #[test]
+    fn it_errors_when_file_is_too_big() -> Result<(), failure::Error> {
+        let mut file = tempfile::NamedTempFile::new()?;
 
-            let data = thread_rng()
-                .sample_iter(&Alphanumeric)
-                .take((2 * MB).try_into().unwrap())
-                .collect::<String>();
+        let data = thread_rng()
+            .sample_iter(&Alphanumeric)
+            .take((2 * MB).try_into().unwrap())
+            .collect::<String>();
 
-            writeln!(file, "{}", data)?;
+        writeln!(file, "{}", data)?;
 
-            let path_buf = file.path().to_path_buf();
+        let path_buf = file.path().to_path_buf();
 
-            assert!(check_file_size(&path_buf, ByteSize::mb(1)).is_err());
+        assert!(check_file_size(&path_buf, ByteSize::mb(1)).is_err());
 
-            Ok(())
-        }
+        Ok(())
     }
 }

src/build/check/wasm.rs

@@ -5,33 +5,30 @@ use wasmparser::Validator;
 use super::{config::V8_SUPPORTED_WASM_FEATURES, Lintable, Parseable, Validate};
 
 #[derive(Debug)]
-pub struct WebAssembly;
+pub struct WebAssembly {
+    bytes: Vec<u8>,
+}
 
 impl Parseable<(PathBuf, Option<PathBuf>)> for WebAssembly {
     fn parse(
-        (_binary_path, _text_path_opt): &(PathBuf, Option<PathBuf>),
+        (binary_path, _text_path_opt): &(PathBuf, Option<PathBuf>),
     ) -> Result<Self, failure::Error> {
-        unimplemented!()
+        let mut bytes: Vec<u8> = Vec::new();
+        File::open(binary_path)?.read_to_end(&mut bytes)?;
+        Ok(Self { bytes })
     }
 }
 
 impl Lintable for WebAssembly {
     fn lint(&self) -> Result<(), failure::Error> {
-        unimplemented!()
-    }
-}
-
-impl Validate<(PathBuf, Option<PathBuf>)> for WebAssembly {
-    fn validate((binary_path, _): (PathBuf, Option<PathBuf>)) -> Result<(), failure::Error> {
         let mut validator = Validator::new();
-        let mut bytes: Vec<u8> = Vec::new();
-
-        File::open(binary_path)?.read_to_end(&mut bytes)?;
         validator.wasm_features(V8_SUPPORTED_WASM_FEATURES);
 
-        match validator.validate_all(&bytes) {
+        match validator.validate_all(&self.bytes) {
             Ok(_) => Ok(()),
             Err(e) => Err(e.into()),
         }
     }
 }
+
+impl Validate<(PathBuf, Option<PathBuf>)> for WebAssembly {}