From b5051e25c219c188f17d499ee4e101a64eb62e37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 19 Aug 2024 21:36:35 +0100 Subject: [PATCH] feat: Deprecate "import assertions" with a warning (#24743) This commit deprecates "import assertions" proposal that has been replaced with "import attributes". Any time an import assertion is encountered a warning will be printed to the terminal. This warning will be printed for both local and remote files (ie. user code and dependencies). Import assertions support will be removed in Deno 2. --- cli/cache/code_cache.rs | 13 +++++++++ cli/main.rs | 5 ++++ cli/module_loader.rs | 28 +++++++++++++++++++ cli/standalone/mod.rs | 1 + runtime/shared.rs | 23 +++++++++++++++ runtime/web_worker.rs | 8 ++++++ runtime/worker.rs | 20 ++++++------- .../future/import_assertions/__test__.jsonc | 9 ++++++ .../future/import_assertions/success.out | 6 ++++ 9 files changed, 102 insertions(+), 11 deletions(-) diff --git a/cli/cache/code_cache.rs b/cli/cache/code_cache.rs index abcd0d46ac..d9e951af6e 100644 --- a/cli/cache/code_cache.rs +++ b/cli/cache/code_cache.rs @@ -80,6 +80,10 @@ impl CodeCache { data, )); } + + pub fn remove_code_cache(&self, specifier: &str) { + Self::ensure_ok(self.inner.remove_code_cache(specifier)) + } } impl code_cache::CodeCache for CodeCache { @@ -158,6 +162,15 @@ impl CodeCacheInner { self.conn.execute(sql, params)?; Ok(()) } + + pub fn remove_code_cache(&self, specifier: &str) -> Result<(), AnyError> { + let sql = " + DELETE FROM codecache + WHERE specifier=$1;"; + let params = params![specifier]; + self.conn.execute(sql, params)?; + Ok(()) + } } fn serialize_code_cache_type( diff --git a/cli/main.rs b/cli/main.rs index aafa7f009f..6a7575deec 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -438,11 +438,15 @@ fn resolve_flags_and_init( DenoSubcommand::Lsp => vec!["--max-old-space-size=3072".to_string()], _ => { if *DENO_FUTURE { + // TODO(bartlomieju): I think this can be removed as it's handled by `deno_core` + // and its settings. // deno_ast removes TypeScript `assert` keywords, so this flag only affects JavaScript // TODO(petamoriken): Need to check TypeScript `assert` keywords in deno_ast vec!["--no-harmony-import-assertions".to_string()] } else { vec![ + // TODO(bartlomieju): I think this can be removed as it's handled by `deno_core` + // and its settings. // If we're still in v1.X version we want to support import assertions. // V8 12.6 unshipped the support by default, so force it by passing a // flag. @@ -455,6 +459,7 @@ fn resolve_flags_and_init( }; init_v8_flags(&default_v8_flags, &flags.v8_flags, get_v8_flags_from_env()); + // TODO(bartlomieju): remove last argument in Deno 2. deno_core::JsRuntime::init_platform(None, !*DENO_FUTURE); util::logger::init(flags.log_level); diff --git a/cli/module_loader.rs b/cli/module_loader.rs index afd707ad82..9f52089363 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use std::cell::RefCell; +use std::collections::HashSet; use std::path::PathBuf; use std::pin::Pin; use std::rc::Rc; @@ -44,6 +45,7 @@ use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::futures::future::FutureExt; use deno_core::futures::Future; +use deno_core::parking_lot::Mutex; use deno_core::resolve_url; use deno_core::ModuleCodeString; use deno_core::ModuleLoader; @@ -291,6 +293,7 @@ impl CliModuleLoaderFactory { emitter: self.shared.emitter.clone(), parsed_source_cache: self.shared.parsed_source_cache.clone(), shared: self.shared.clone(), + prevent_v8_code_cache: Default::default(), }))); ModuleLoaderAndSourceMapGetter { module_loader: loader, @@ -342,6 +345,10 @@ struct CliModuleLoaderInner { emitter: Arc, parsed_source_cache: Arc, graph_container: TGraphContainer, + // NOTE(bartlomieju): this is temporary, for deprecated import assertions. + // Should be removed in Deno 2. + // Modules stored here should not be V8 code-cached. + prevent_v8_code_cache: Arc>>, } impl @@ -827,6 +834,14 @@ impl ModuleLoader code_cache: &[u8], ) -> Pin>> { if let Some(cache) = self.0.shared.code_cache.as_ref() { + if self + .0 + .prevent_v8_code_cache + .lock() + .contains(specifier.as_str()) + { + return std::future::ready(()).boxed_local(); + } // This log line is also used by tests. log::debug!( "Updating V8 code cache for ES module: {specifier}, [{source_hash:?}]" @@ -841,6 +856,19 @@ impl ModuleLoader std::future::ready(()).boxed_local() } + fn purge_and_prevent_code_cache(&self, specifier: &str) { + if let Some(cache) = self.0.shared.code_cache.as_ref() { + // This log line is also used by tests. + log::debug!("Remove V8 code cache for ES module: {specifier}"); + cache.remove_code_cache(specifier); + self + .0 + .prevent_v8_code_cache + .lock() + .insert(specifier.to_string()); + } + } + fn get_source_map(&self, file_name: &str) -> Option> { let specifier = resolve_url(file_name).ok()?; match specifier.scheme() { diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 635293cc94..020561ece2 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -745,6 +745,7 @@ pub async fn run( // Initialize v8 once from the main thread. v8_set_flags(construct_v8_flags(&[], &metadata.v8_flags, vec![])); + // TODO(bartlomieju): remove last argument in Deno 2. deno_core::JsRuntime::init_platform(None, true); let mut worker = worker_factory diff --git a/runtime/shared.rs b/runtime/shared.rs index 1b2136c638..c52521690c 100644 --- a/runtime/shared.rs +++ b/runtime/shared.rs @@ -116,3 +116,26 @@ pub fn maybe_transpile_source( Ok((source_text.into(), maybe_source_map)) } + +pub fn import_assertion_callback( + args: deno_core::ImportAssertionsSupportCustomCallbackArgs, +) { + let mut msg = deno_terminal::colors::yellow("⚠️ Import assertions are deprecated. Use `with` keyword, instead of 'assert' keyword.").to_string(); + if let Some(specifier) = args.maybe_specifier { + if let Some(source_line) = args.maybe_source_line { + msg.push_str("\n\n"); + msg.push_str(&source_line); + msg.push_str("\n\n"); + } + msg.push_str(&format!( + " at {}:{}:{}\n", + specifier, + args.maybe_line_number.unwrap(), + args.column_number + )); + #[allow(clippy::print_stderr)] + { + eprintln!("{}", msg); + } + } +} diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index ed1e19c9ec..3e95045db4 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -538,6 +538,13 @@ impl WebWorker { options.bootstrap.enable_op_summary_metrics, options.strace_ops, ); + let import_assertions_support = if options.bootstrap.future { + deno_core::ImportAssertionsSupport::Error + } else { + deno_core::ImportAssertionsSupport::CustomCallback(Box::new( + crate::shared::import_assertion_callback, + )) + }; let mut js_runtime = JsRuntime::new(RuntimeOptions { module_loader: Some(options.module_loader.clone()), @@ -558,6 +565,7 @@ impl WebWorker { validate_import_attributes_cb: Some(Box::new( validate_import_attributes_callback, )), + import_assertions_support, ..Default::default() }); diff --git a/runtime/worker.rs b/runtime/worker.rs index 696786b564..c1c918d083 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -476,6 +476,14 @@ impl MainWorker { } }); + let import_assertions_support = if options.bootstrap.future { + deno_core::ImportAssertionsSupport::Error + } else { + deno_core::ImportAssertionsSupport::CustomCallback(Box::new( + crate::shared::import_assertion_callback, + )) + }; + let mut js_runtime = JsRuntime::new(RuntimeOptions { module_loader: Some(options.module_loader.clone()), startup_snapshot: options.startup_snapshot, @@ -501,6 +509,7 @@ impl MainWorker { validate_import_attributes_cb: Some(Box::new( validate_import_attributes_callback, )), + import_assertions_support, eval_context_code_cache_cbs: options.v8_code_cache.map(|cache| { let cache_clone = cache.clone(); ( @@ -544,17 +553,6 @@ impl MainWorker { if let Some(op_summary_metrics) = op_summary_metrics { js_runtime.op_state().borrow_mut().put(op_summary_metrics); } - extern "C" fn message_handler( - _msg: v8::Local, - _exception: v8::Local, - ) { - // TODO(@littledivy): Propogate message to users. - } - - // Register message listener - js_runtime - .v8_isolate() - .add_message_listener(message_handler); if let Some(server) = options.maybe_inspector_server.clone() { server.register_inspector( diff --git a/tests/specs/future/import_assertions/__test__.jsonc b/tests/specs/future/import_assertions/__test__.jsonc index a1e759c75e..1c55d2220e 100644 --- a/tests/specs/future/import_assertions/__test__.jsonc +++ b/tests/specs/future/import_assertions/__test__.jsonc @@ -1,5 +1,14 @@ { "steps": [ + { + "args": "run main.js", + "output": "error.out", + "exitCode": 1, + "envs": { + "DENO_FUTURE": "1" + } + }, + // Running the same multiple times, should warn each time. { "args": "run main.js", "output": "error.out", diff --git a/tests/specs/future/import_assertions/success.out b/tests/specs/future/import_assertions/success.out index 70ec274d9a..fcf28b9436 100644 --- a/tests/specs/future/import_assertions/success.out +++ b/tests/specs/future/import_assertions/success.out @@ -1 +1,7 @@ +⚠️ Import assertions are deprecated. Use `with` keyword, instead of 'assert' keyword. + +import foo from "./main.json" assert { type: "json" }; + + at [WILDCARD]import_assertions/main.js:1:30 + { foo: "foo" }