From ab898556a490d78d93e806ea3ee31147117102e3 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Fri, 23 Oct 2020 22:05:41 +1100 Subject: [PATCH] refactor(cli): move bundle check to new infrastructure (#8071) --- cli/main.rs | 70 ++++++---- cli/module_graph2.rs | 2 +- cli/tests/bundle.test.out | 43 +++--- cli/tests/bundle_dynamic_import.ts | 3 + cli/tests/integration_tests.rs | 7 +- cli/tests/lock_check_err_with_bundle.out | 5 +- cli/tsc.rs | 171 ----------------------- cli/tsc/99_main_compiler.js | 161 --------------------- 8 files changed, 79 insertions(+), 383 deletions(-) create mode 100644 cli/tests/bundle_dynamic_import.ts diff --git a/cli/main.rs b/cli/main.rs index 51355555ee..4469fe57c6 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -67,6 +67,7 @@ use crate::permissions::Permissions; use crate::program_state::ProgramState; use crate::specifier_handler::FetchHandler; use crate::worker::MainWorker; +use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::futures::future::FutureExt; use deno_core::futures::Future; @@ -311,38 +312,57 @@ async fn bundle_command( module_specifier.to_string() ); - let output = if flags.no_check { - let handler = Rc::new(RefCell::new(FetchHandler::new( - &program_state, - // when bundling, dynamic imports are only access for their type safety, - // therefore we will allow the graph to access any module. - Permissions::allow_all(), - )?)); - let mut builder = module_graph2::GraphBuilder2::new( - handler, - program_state.maybe_import_map.clone(), - ); - builder.add(&module_specifier, false).await?; - let graph = builder.get_graph(&program_state.lockfile); + let handler = Rc::new(RefCell::new(FetchHandler::new( + &program_state, + // when bundling, dynamic imports are only access for their type safety, + // therefore we will allow the graph to access any module. + Permissions::allow_all(), + )?)); + let mut builder = module_graph2::GraphBuilder2::new( + handler, + program_state.maybe_import_map.clone(), + ); + builder.add(&module_specifier, false).await?; + let graph = builder.get_graph(&program_state.lockfile); - let (s, stats, maybe_ignored_options) = - graph.bundle(module_graph2::BundleOptions { - debug: flags.log_level == Some(Level::Debug), - maybe_config_path: flags.config_path, + let debug = flags.log_level == Some(log::Level::Debug); + if !flags.no_check { + // TODO(@kitsonk) support bundling for workers + let lib = if flags.unstable { + module_graph2::TypeLib::UnstableDenoWindow + } else { + module_graph2::TypeLib::DenoWindow + }; + let graph = graph.clone(); + let (stats, diagnostics, maybe_ignored_options) = + graph.check(module_graph2::CheckOptions { + debug, + emit: false, + lib, + maybe_config_path: flags.config_path.clone(), + reload: flags.reload, })?; + debug!("{}", stats); if let Some(ignored_options) = maybe_ignored_options { eprintln!("{}", ignored_options); } - debug!("{}", stats); + if !diagnostics.0.is_empty() { + return Err(generic_error(diagnostics.to_string())); + } + } - s - } else { - program_state - .ts_compiler - .bundle(&program_state, module_specifier) - .await? - }; + let (output, stats, maybe_ignored_options) = + graph.bundle(module_graph2::BundleOptions { + debug, + maybe_config_path: flags.config_path, + })?; + + if flags.no_check && maybe_ignored_options.is_some() { + let ignored_options = maybe_ignored_options.unwrap(); + eprintln!("{}", ignored_options); + } + debug!("{}", stats); debug!(">>>>> bundle END"); diff --git a/cli/module_graph2.rs b/cli/module_graph2.rs index 4895cc53ae..fd4d464610 100644 --- a/cli/module_graph2.rs +++ b/cli/module_graph2.rs @@ -575,7 +575,7 @@ pub struct TranspileOptions { /// A dependency graph of modules, were the modules that have been inserted via /// the builder will be loaded into the graph. Also provides an interface to /// be able to manipulate and handle the graph. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Graph2 { /// A reference to the specifier handler that will retrieve and cache modules /// for the graph. diff --git a/cli/tests/bundle.test.out b/cli/tests/bundle.test.out index eba439424c..d90dc9f95e 100644 --- a/cli/tests/bundle.test.out +++ b/cli/tests/bundle.test.out @@ -1,22 +1,23 @@ [WILDCARD] -let System, __instantiate; -(() => { -[WILDCARD] -})(); - -System.register("print_hello", [], function (exports_1, context_1) { -[WILDCARD] -}); -System.register("subdir2/mod2", ["print_hello"], function (exports_2, context_2) { -[WILDCARD] -}); -System.register("mod1", ["subdir2/mod2"], function (exports_3, context_3) { -[WILDCARD] -}); - -const __exp = __instantiate("mod1", false); -export const returnsHi = __exp["returnsHi"]; -export const returnsFoo2 = __exp["returnsFoo2"]; -export const printHello3 = __exp["printHello3"]; -export const throwsError = __exp["throwsError"]; -[WILDCARD] +function printHello() { + console.log("Hello"); +} +function returnsFoo() { + return "Foo"; +} +function printHello2() { + printHello(); +} +export function returnsHi() { + return "Hi"; +} +export function returnsFoo2() { + return returnsFoo(); +} +export function printHello3() { + printHello2(); +} +export function throwsError() { + throw Error("exception from mod1"); +} +[WILDCARD] \ No newline at end of file diff --git a/cli/tests/bundle_dynamic_import.ts b/cli/tests/bundle_dynamic_import.ts new file mode 100644 index 0000000000..7548316327 --- /dev/null +++ b/cli/tests/bundle_dynamic_import.ts @@ -0,0 +1,3 @@ +const mod1 = await import("http://localhost:4545/cli/tests/subdir/mod1.ts"); + +mod1.printHello3(); diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index b9264aa0f5..11240e71bf 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -1017,11 +1017,12 @@ fn bundle_js() { #[test] fn bundle_dynamic_import() { + let _g = util::http_server(); let dynamic_import = - util::root_path().join("cli/tests/subdir/subdir2/dynamic_import.ts"); + util::root_path().join("cli/tests/bundle_dynamic_import.ts"); assert!(dynamic_import.is_file()); let t = TempDir::new().expect("tempdir fail"); - let bundle = t.path().join("dynamic_import.bundle.js"); + let bundle = t.path().join("bundle_dynamic_import.bundle.js"); let mut deno = util::deno_cmd() .current_dir(util::root_path()) .arg("bundle") @@ -1036,6 +1037,8 @@ fn bundle_dynamic_import() { let output = util::deno_cmd() .current_dir(util::root_path()) .arg("run") + .arg("--allow-net") + .arg("--quiet") .arg(&bundle) .output() .expect("failed to spawn script"); diff --git a/cli/tests/lock_check_err_with_bundle.out b/cli/tests/lock_check_err_with_bundle.out index ce0e029efe..d6eae26b1f 100644 --- a/cli/tests/lock_check_err_with_bundle.out +++ b/cli/tests/lock_check_err_with_bundle.out @@ -1,3 +1,4 @@ [WILDCARD] -Subresource integrity check failed --lock=lock_check_err_with_bundle.json -http://127.0.0.1:4545/cli/tests/subdir/subdir2/mod2.ts +The source code is invalid, as it does not match the expected hash in the lock file. + Specifier: http://127.0.0.1:4545/cli/tests/subdir/subdir2/mod2.ts + Lock file: lock_check_err_with_bundle.json diff --git a/cli/tsc.rs b/cli/tsc.rs index dfd733d48b..796b585feb 100644 --- a/cli/tsc.rs +++ b/cli/tsc.rs @@ -27,7 +27,6 @@ use deno_core::JsRuntime; use deno_core::ModuleSpecifier; use deno_core::RuntimeOptions; use log::debug; -use log::Level; use regex::Regex; use serde::Deserialize; use serde::Serialize; @@ -136,14 +135,6 @@ lazy_static! { Regex::new(r#"(?i)\slib\s*=\s*["']([^"']*)["']"#).unwrap(); } -fn warn_ignored_options( - maybe_ignored_options: Option, -) { - if let Some(ignored_options) = maybe_ignored_options { - eprintln!("{}", ignored_options); - } -} - #[derive(Clone)] pub enum TargetLib { Main, @@ -243,15 +234,6 @@ fn source_code_version_hash( crate::checksum::gen(&[source_code, version.as_bytes(), config_hash]) } -fn maybe_log_stats(maybe_stats: Option>) { - if let Some(stats) = maybe_stats { - debug!("DEBUG - Compilation Statistics:"); - for stat in stats { - debug!("{}: {}", stat.key, stat.value); - } - } -} - pub struct TsCompilerInner { pub file_fetcher: SourceFileFetcher, pub flags: Flags, @@ -291,14 +273,6 @@ struct EmittedSource { contents: String, } -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct BundleResponse { - diagnostics: Diagnostics, - bundle_output: Option, - stats: Option>, -} - // TODO(bartlomieju): possible deduplicate once TS refactor is stabilized #[derive(Deserialize)] #[serde(rename_all = "camelCase")] @@ -342,121 +316,6 @@ impl TsCompiler { c.insert(url.clone()); } - /// For a given module, generate a single file JavaScript output that includes - /// all the dependencies for that module. - pub async fn bundle( - &self, - program_state: &Arc, - module_specifier: ModuleSpecifier, - ) -> Result { - debug!( - "Invoking the compiler to bundle. module_name: {}", - module_specifier.to_string() - ); - - let permissions = Permissions::allow_all(); - let mut module_graph_loader = ModuleGraphLoader::new( - self.file_fetcher.clone(), - program_state.maybe_import_map.clone(), - permissions.clone(), - false, - true, - ); - module_graph_loader - .add_to_graph(&module_specifier, None) - .await?; - let module_graph = module_graph_loader.get_graph(); - let module_graph_files = module_graph.values().collect::>(); - // Check integrity of every file in module graph - if let Some(ref lockfile) = program_state.lockfile { - let mut g = lockfile.lock().unwrap(); - - for graph_file in &module_graph_files { - let check_passed = - g.check_or_insert(&graph_file.url, &graph_file.source_code); - - if !check_passed { - eprintln!( - "Subresource integrity check failed --lock={}\n{}", - g.filename.display(), - graph_file.url - ); - std::process::exit(10); - } - } - } - if let Some(ref lockfile) = program_state.lockfile { - let g = lockfile.lock().unwrap(); - g.write()?; - } - let module_graph_json = - serde_json::to_value(module_graph).expect("Failed to serialize data"); - - let root_names = vec![module_specifier.to_string()]; - let target = "main"; - let performance = - matches!(program_state.flags.log_level, Some(Level::Debug)); - let unstable = self.flags.unstable; - - let mut lib = if target == "main" { - vec!["deno.window"] - } else { - vec!["deno.worker"] - }; - - if unstable { - lib.push("deno.unstable"); - } - - let mut compiler_options = json!({ - "allowJs": true, - "allowNonTsExtensions": true, - "checkJs": false, - "esModuleInterop": true, - "inlineSourceMap": false, - "jsx": "react", - "lib": lib, - "module": "system", - "outFile": "deno:///bundle.js", - // disabled until we have effective way to modify source maps - "sourceMap": false, - "strict": true, - "removeComments": true, - "target": "esnext", - }); - - let compiler_config = self.config.clone(); - - tsc_config::json_merge(&mut compiler_options, &compiler_config.options); - - warn_ignored_options(compiler_config.maybe_ignored_options); - - let j = json!({ - "type": CompilerRequestType::Bundle, - "target": target, - "rootNames": root_names, - "performance": performance, - "compilerOptions": compiler_options, - "sourceFileMap": module_graph_json, - }); - - let req_msg = j.to_string(); - - let json_str = execute_in_tsc(program_state.clone(), req_msg)?; - - let bundle_response: BundleResponse = serde_json::from_str(&json_str)?; - - maybe_log_stats(bundle_response.stats); - - if !bundle_response.diagnostics.0.is_empty() { - return Err(generic_error(bundle_response.diagnostics.to_string())); - } - - assert!(bundle_response.bundle_output.is_some()); - let output = bundle_response.bundle_output.unwrap(); - Ok(output) - } - fn cache_emitted_files( &self, emit_map: HashMap, @@ -1033,7 +892,6 @@ fn parse_deno_types(comment: &str) -> Option { #[repr(i32)] #[derive(Clone, Copy, PartialEq, Debug)] pub enum CompilerRequestType { - Bundle = 1, RuntimeCompile = 2, RuntimeBundle = 3, RuntimeTranspile = 4, @@ -1045,7 +903,6 @@ impl Serialize for CompilerRequestType { S: Serializer, { let value: i32 = match self { - CompilerRequestType::Bundle => 1 as i32, CompilerRequestType::RuntimeCompile => 2 as i32, CompilerRequestType::RuntimeBundle => 3 as i32, CompilerRequestType::RuntimeTranspile => 4 as i32, @@ -1058,7 +915,6 @@ impl Serialize for CompilerRequestType { mod tests { use super::*; use crate::fs as deno_fs; - use crate::program_state::ProgramState; use tempfile::TempDir; #[test] @@ -1118,33 +974,6 @@ mod tests { assert!(parse_ts_reference(r#"/ "#).is_none()); } - #[tokio::test] - async fn test_bundle() { - let p = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .parent() - .unwrap() - .join("cli/tests/002_hello.ts"); - use deno_core::ModuleSpecifier; - let module_name = - ModuleSpecifier::resolve_url_or_path(p.to_str().unwrap()).unwrap(); - - let mock_state = ProgramState::mock( - vec![ - String::from("deno"), - p.to_string_lossy().into(), - String::from("$deno$/bundle.js"), - ], - None, - ); - - let result = mock_state - .ts_compiler - .bundle(&mock_state, module_name) - .await; - - assert!(result.is_ok()); - } - #[test] fn test_source_code_version_hash() { assert_eq!( diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index 86a68a6bd4..ee03ff6ff9 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -594,68 +594,9 @@ delete Object.prototype.__proto__; } } - function buildSourceFileCache(sourceFileMap) { - for (const entry of Object.values(sourceFileMap)) { - SourceFile.addToCache({ - url: entry.url, - filename: entry.url, - mediaType: entry.mediaType, - sourceCode: entry.sourceCode, - versionHash: entry.versionHash, - }); - - for (const importDesc of entry.imports) { - let mappedUrl = importDesc.resolvedSpecifier; - const importedFile = sourceFileMap[importDesc.resolvedSpecifier]; - // IMPORTANT: due to HTTP redirects we might end up in situation - // where URL points to a file with completely different URL. - // In that case we take value of `redirect` field and cache - // resolved specifier pointing to the value of the redirect. - // It's not very elegant solution and should be rethinked. - assert(importedFile); - if (importedFile.redirect) { - mappedUrl = importedFile.redirect; - } - const isJsOrJsx = importedFile.mediaType === MediaType.JavaScript || - importedFile.mediaType === MediaType.JSX; - // If JS or JSX perform substitution for types if available - if (isJsOrJsx) { - // @deno-types has highest precedence, followed by - // X-TypeScript-Types header - if (importDesc.resolvedTypeDirective) { - mappedUrl = importDesc.resolvedTypeDirective; - } else if (importedFile.typeHeaders.length > 0) { - const typeHeaders = importedFile.typeHeaders[0]; - mappedUrl = typeHeaders.resolvedSpecifier; - } else if (importedFile.typesDirectives.length > 0) { - const typeDirective = importedFile.typesDirectives[0]; - mappedUrl = typeDirective.resolvedSpecifier; - } - } - - SourceFile.cacheResolvedUrl(mappedUrl, importDesc.specifier, entry.url); - } - for (const fileRef of entry.referencedFiles) { - SourceFile.cacheResolvedUrl( - fileRef.resolvedSpecifier, - fileRef.specifier, - entry.url, - ); - } - for (const fileRef of entry.libDirectives) { - SourceFile.cacheResolvedUrl( - fileRef.resolvedSpecifier, - fileRef.specifier, - entry.url, - ); - } - } - } - // Warning! The values in this enum are duplicated in `cli/msg.rs` // Update carefully! const CompilerRequestType = { - Bundle: 1, RuntimeCompile: 2, RuntimeBundle: 3, RuntimeTranspile: 4, @@ -945,103 +886,6 @@ delete Object.prototype.__proto__; .map((sym) => sym.getName()); } - function bundle({ - compilerOptions, - rootNames, - target, - sourceFileMap, - type, - performance, - }) { - if (performance) { - performanceStart(); - } - debug(">>> bundle start", { - rootNames, - type: CompilerRequestType[type], - }); - - // When a programme is emitted, TypeScript will call `writeFile` with - // each file that needs to be emitted. The Deno compiler host delegates - // this, to make it easier to perform the right actions, which vary - // based a lot on the request. - const state = { - rootNames, - bundleOutput: undefined, - }; - - const { options, diagnostics: diags } = parseCompilerOptions( - compilerOptions, - ); - - diagnostics = diags.filter( - ({ code }) => code != 5023 && !IGNORED_DIAGNOSTICS.includes(code), - ); - - // TODO(bartlomieju): this options is excluded by `ts.convertCompilerOptionsFromJson` - // however stuff breaks if it's not passed (type_directives_js_main.js) - options.allowNonTsExtensions = true; - - legacyHostState.target = target; - legacyHostState.writeFile = createBundleWriteFile(state); - state.options = options; - - buildSourceFileCache(sourceFileMap); - // if there was a configuration and no diagnostics with it, we will continue - // to generate the program and possibly emit it. - if (diagnostics.length === 0) { - const program = ts.createProgram({ - rootNames, - options, - host, - }); - - diagnostics = ts - .getPreEmitDiagnostics(program) - .filter(({ code }) => !IGNORED_DIAGNOSTICS.includes(code)); - - // We will only proceed with the emit if there are no diagnostics. - if (diagnostics.length === 0) { - // we only support a single root module when bundling - assert(rootNames.length === 1); - setRootExports(program, rootNames[0]); - const emitResult = program.emit(); - assert( - emitResult.emitSkipped === false, - "Unexpected skip of the emit.", - ); - // emitResult.diagnostics is `readonly` in TS3.5+ and can't be assigned - // without casting. - diagnostics = emitResult.diagnostics; - } - if (performance) { - performanceProgram({ program }); - } - } - - let bundleOutput; - - if (diagnostics.length === 0) { - assert(state.bundleOutput); - bundleOutput = state.bundleOutput; - } - - const stats = performance ? performanceEnd() : undefined; - - const result = { - bundleOutput, - diagnostics: fromTypeScriptDiagnostic(diagnostics), - stats, - }; - - debug("<<< bundle end", { - rootNames, - type: CompilerRequestType[type], - }); - - return result; - } - function runtimeCompile(request) { const { compilerOptions, rootNames, target, sourceFileMap } = request; @@ -1187,11 +1031,6 @@ delete Object.prototype.__proto__; function tsCompilerOnMessage(msg) { const request = msg.data; switch (request.type) { - case CompilerRequestType.Bundle: { - const result = bundle(request); - opCompilerRespond(result); - break; - } case CompilerRequestType.RuntimeCompile: { const result = runtimeCompile(request); opCompilerRespond(result);