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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"foo": "foo"
}
1 change: 1 addition & 0 deletions tests/specs/future/import_assertions/success.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ foo: "foo" }
2 changes: 1 addition & 1 deletion tests/testdata/npm/import_json/main.js
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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