Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FUTURE: remove import assertions support for JavaScript #23541

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/args/mod.rs
Expand Up @@ -110,7 +110,7 @@ pub static DENO_DISABLE_PEDANTIC_NODE_WARNINGS: Lazy<bool> = Lazy::new(|| {
.is_some()
});

static DENO_FUTURE: Lazy<bool> =
pub static DENO_FUTURE: Lazy<bool> =
Lazy::new(|| std::env::var("DENO_FUTURE").ok().is_some());

pub fn jsr_url() -> &'static Url {
Expand Down
3 changes: 2 additions & 1 deletion cli/lsp/config.rs
Expand Up @@ -4,6 +4,7 @@ use super::logging::lsp_log;
use crate::args::ConfigFile;
use crate::args::FmtOptions;
use crate::args::LintOptions;
use crate::args::DENO_FUTURE;
use crate::cache::FastInsecureHasher;
use crate::file_fetcher::FileFetcher;
use crate::lsp::logging::lsp_warn;
Expand Down Expand Up @@ -1380,7 +1381,7 @@ impl ConfigData {
.as_ref()
.map(|c| c.has_unstable("byonm"))
.unwrap_or(false)
|| (std::env::var("DENO_FUTURE").is_ok()
|| (*DENO_FUTURE
&& package_json.is_some()
&& config_file
.as_ref()
Expand Down
11 changes: 10 additions & 1 deletion cli/main.rs
Expand Up @@ -29,6 +29,7 @@ mod worker;
use crate::args::flags_from_vec;
use crate::args::DenoSubcommand;
use crate::args::Flags;
use crate::args::DENO_FUTURE;
use crate::util::display;
use crate::util::v8::get_v8_flags_from_env;
use crate::util::v8::init_v8_flags;
Expand Down Expand Up @@ -389,7 +390,15 @@ fn resolve_flags_and_init(
// Using same default as VSCode:
// https://github.com/microsoft/vscode/blob/48d4ba271686e8072fc6674137415bc80d936bc7/extensions/typescript-language-features/src/configuration/configuration.ts#L213-L214
DenoSubcommand::Lsp => vec!["--max-old-space-size=3072".to_string()],
_ => vec![],
_ => {
if *DENO_FUTURE {
// deno_ast removes TypeScript `assert` keywords, so this flag only affects JavaScript
// TODO(petamoriken): Need to check TypeScript `assert` keywords in deno_ast
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should address this one before landing the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. It would be better to work on a separate PR for TypeScript, since it is likely that the error will be displayed on the deno_ast side (swc-project/swc#8893 (comment)), rather than using this flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me talk with @dsherret about this first 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe we should just not worry about it for now? It would be a lot of work and supporting it can make more code just keep working for the time being.

vec!["--no-harmony-import-assertions".to_string()]
} else {
vec![]
}
}
};

init_v8_flags(&default_v8_flags, &flags.v8_flags, get_v8_flags_from_env());
Expand Down
10 changes: 0 additions & 10 deletions tests/integration/run_tests.rs
Expand Up @@ -1694,16 +1694,6 @@ fn type_directives_js_main() {
assert_not_contains!(output.combined_output(), "type_reference.d.ts");
}

#[test]
fn test_deno_futures_env() {
let context = TestContextBuilder::new().add_future_env_vars().build();
let output = context
.new_command()
.args("run --quiet --reload run/deno_futures_env.ts")
.run();
output.assert_exit_code(0);
}

itest!(type_directives_redirect {
args: "run --reload --check run/type_directives_redirect.ts",
output: "run/type_directives_redirect.ts.out",
Expand Down
16 changes: 16 additions & 0 deletions tests/specs/future/import_assertions/__test__.jsonc
@@ -0,0 +1,16 @@
{
"steps": [
{
"args": "run main.js",
"output": "error.out",
"exitCode": 1,
"envs": {
"DENO_FUTURE": "1"
}
},
{
"args": "run main.js",
"output": "success.out"
}
]
}
4 changes: 4 additions & 0 deletions tests/specs/future/import_assertions/error.out
@@ -0,0 +1,4 @@
error: Uncaught SyntaxError: Unexpected identifier 'assert'
import foo from "./main.json" assert { type: "json" };
^
at <anonymous> (file:///[WILDCARD]/main.js:[WILDCARD])
2 changes: 2 additions & 0 deletions tests/specs/future/import_assertions/main.js
@@ -0,0 +1,2 @@
import foo from "./main.json" assert { type: "json" };
console.log(foo);
3 changes: 3 additions & 0 deletions tests/specs/future/import_assertions/main.json
@@ -0,0 +1,3 @@
{
"foo": "foo"
}
1 change: 1 addition & 0 deletions tests/specs/future/import_assertions/success.out
@@ -0,0 +1 @@
{ foo: "foo" }
2 changes: 1 addition & 1 deletion tests/testdata/npm/import_json/main.js
@@ -1,4 +1,4 @@
import json from "npm:@denotest/binary-package@1/package.json" assert {
import json from "npm:@denotest/binary-package@1/package.json" with {
type: "json",
};
console.log(json);
3 changes: 0 additions & 3 deletions tests/testdata/run/deno_futures_env.ts

This file was deleted.

2 changes: 1 addition & 1 deletion tests/testdata/subdir/mod7.js
@@ -1,3 +1,3 @@
import json1 from "./json_1.json" assert { type: "json" };
import json1 from "./json_1.json" with { type: "json" };

console.log(json1);
2 changes: 1 addition & 1 deletion tests/testdata/subdir/mod8.js
@@ -1,3 +1,3 @@
import json3 from "./json_3.json" assert { type: "json" };
import json3 from "./json_3.json" with { type: "json" };

console.log(json3);
2 changes: 1 addition & 1 deletion tools/napi/generate_symbols_lists.js
@@ -1,7 +1,7 @@
#!/usr/bin/env -S deno run --allow-read --allow-write
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

import exports from "../../cli/napi/sym/symbol_exports.json" assert {
import exports from "../../cli/napi/sym/symbol_exports.json" with {
type: "json",
};

Expand Down