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

BREAKING: remove support for JSON imports #5037

Merged
merged 1 commit into from
May 1, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 0 additions & 50 deletions cli/compilers/json.rs

This file was deleted.

2 changes: 0 additions & 2 deletions cli/compilers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ use futures::Future;

mod compiler_worker;
mod js;
mod json;
mod ts;
mod wasm;

pub use js::JsCompiler;
pub use json::JsonCompiler;
pub use ts::runtime_compile;
pub use ts::runtime_transpile;
pub use ts::TargetLib;
Expand Down
8 changes: 3 additions & 5 deletions cli/global_state.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
use crate::compilers::CompiledModule;
use crate::compilers::JsCompiler;
use crate::compilers::JsonCompiler;
use crate::compilers::TargetLib;
use crate::compilers::TsCompiler;
use crate::compilers::WasmCompiler;
Expand Down Expand Up @@ -36,7 +35,6 @@ pub struct GlobalStateInner {
pub dir: deno_dir::DenoDir,
pub file_fetcher: SourceFileFetcher,
pub js_compiler: JsCompiler,
pub json_compiler: JsonCompiler,
pub ts_compiler: TsCompiler,
pub wasm_compiler: WasmCompiler,
pub lockfile: Option<Mutex<Lockfile>>,
Expand Down Expand Up @@ -88,7 +86,6 @@ impl GlobalState {
file_fetcher,
ts_compiler,
js_compiler: JsCompiler {},
json_compiler: JsonCompiler {},
wasm_compiler: WasmCompiler::default(),
lockfile,
compiler_starts: AtomicUsize::new(0),
Expand Down Expand Up @@ -118,8 +115,9 @@ impl GlobalState {
let compile_lock = self.compile_lock.lock().await;

let compiled_module = match out.media_type {
msg::MediaType::Unknown => state1.js_compiler.compile(out).await,
msg::MediaType::Json => state1.json_compiler.compile(&out).await,
msg::MediaType::Json | msg::MediaType::Unknown => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I think MediaType::Json could now be removed; it'd be handled as MediaType::Unknown

state1.js_compiler.compile(out).await
}
msg::MediaType::Wasm => {
state1.wasm_compiler.compile(state1.clone(), &out).await
}
Expand Down
2 changes: 0 additions & 2 deletions cli/js/compiler/imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ function getMediaType(filename: string): MediaType {
return MediaType.JavaScript;
case "jsx":
return MediaType.JSX;
case "json":
return MediaType.Json;
case "ts":
return MediaType.TypeScript;
case "tsx":
Expand Down
5 changes: 0 additions & 5 deletions cli/js/compiler/sourcefile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ function getExtension(fileName: string, mediaType: MediaType): ts.Extension {
return fileName.endsWith(".d.ts") ? ts.Extension.Dts : ts.Extension.Ts;
case MediaType.TSX:
return ts.Extension.Tsx;
case MediaType.Json:
// we internally compile JSON, so what gets provided to the TypeScript
// compiler is an ES module, but in order to get TypeScript to handle it
// properly we have to pretend it is TS.
return ts.Extension.Ts;
case MediaType.Wasm:
// Custom marker for Wasm type.
return ts.Extension.Js;
Expand Down
12 changes: 1 addition & 11 deletions cli/js/compiler/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,6 @@ function cache(
const sf = SourceFile.get(moduleId);

if (sf) {
// NOTE: If it's a `.json` file we don't want to write it to disk.
// JSON files are loaded and used by TS compiler to check types, but we don't want
// to emit them to disk because output file is the same as input file.
if (sf.mediaType === MediaType.Json) {
return;
}

// NOTE: JavaScript files are only cached to disk if `checkJs`
// option in on
if (sf.mediaType === MediaType.JavaScript && !checkJs) {
Expand All @@ -65,10 +58,7 @@ function cache(
if (emittedFileName.endsWith(".map")) {
// Source Map
compilerOps.cache(".map", moduleId, contents);
} else if (
emittedFileName.endsWith(".js") ||
emittedFileName.endsWith(".json")
) {
} else if (emittedFileName.endsWith(".js")) {
// Compiled JavaScript
compilerOps.cache(".js", moduleId, contents);
} else {
Expand Down
8 changes: 0 additions & 8 deletions cli/ops/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,6 @@ fn op_fetch_source_files(
.map_err(|e| OpError::other(e.to_string()))?
.code
}
msg::MediaType::Json => {
global_state
.json_compiler
.compile(&file)
.await
.map_err(|e| OpError::other(e.to_string()))?
.code
}
_ => String::from_utf8(file.source_code)
.map_err(|_| OpError::invalid_utf8())?,
};
Expand Down
10 changes: 9 additions & 1 deletion cli/tests/020_json_modules.ts.out
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
{"foo":{"bar":true,"baz":["qat",1]}}
[WILDCARD]
error: Uncaught TypeError: Cannot resolve extension for "[WILDCARD]config.json" with mediaType "Json".
at getExtension ($deno$/compiler/sourcefile.ts:[WILDCARD])
at new SourceFile ($deno$/compiler/sourcefile.ts:[WILDCARD])
at processImports ($deno$/compiler/imports.ts:[WILDCARD])
at async Object.processImports ($deno$/compiler/imports.ts:[WILDCARD])
at async compile ([WILDCARD]compiler.ts:[WILDCARD])
at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD])
at async workerMessageRecvCallback ($deno$/runtime_worker.ts:[WILDCARD])
7 changes: 0 additions & 7 deletions cli/tests/050_more_jsons.ts

This file was deleted.

9 changes: 0 additions & 9 deletions cli/tests/050_more_jsons.ts.out

This file was deleted.

39 changes: 2 additions & 37 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,38 +395,6 @@ fn bundle_single_module() {
assert_eq!(output.stderr, b"");
}

#[test]
fn bundle_json() {
let json_modules = util::root_path().join("cli/tests/020_json_modules.ts");
assert!(json_modules.is_file());
let t = TempDir::new().expect("tempdir fail");
let bundle = t.path().join("020_json_modules.bundle.js");
let mut deno = util::deno_cmd()
.current_dir(util::root_path())
.arg("bundle")
.arg(json_modules)
.arg(&bundle)
.spawn()
Comment on lines -398 to -409
Copy link
Member Author

Choose a reason for hiding this comment

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

@ry thanks for pointing this out; I guess there should be permission check involved in this case like a regular disk read?

.expect("failed to spawn script");
let status = deno.wait().expect("failed to wait for the child process");
assert!(status.success());
assert!(bundle.is_file());

let output = util::deno_cmd()
.current_dir(util::root_path())
.arg("run")
.arg("--reload")
.arg(&bundle)
.output()
.expect("failed to spawn script");
// check the output of the the bundle program.
assert!(std::str::from_utf8(&output.stdout)
.unwrap()
.trim()
.ends_with("{\"foo\":{\"bar\":true,\"baz\":[\"qat\",1]}}"));
assert_eq!(output.stderr, b"");
}

#[test]
fn bundle_tla() {
// First we have to generate a bundle of some module that has exports.
Expand Down Expand Up @@ -927,7 +895,9 @@ itest_ignore!(_019_media_types {

itest!(_020_json_modules {
args: "run --reload 020_json_modules.ts",
check_stderr: true,
output: "020_json_modules.ts.out",
exit_code: 1,
});

itest!(_021_mjs_modules {
Expand Down Expand Up @@ -1127,11 +1097,6 @@ itest_ignore!(_049_info_flag_script_jsx {
http_server: true,
});

itest!(_050_more_jsons {
args: "run --reload 050_more_jsons.ts",
output: "050_more_jsons.ts.out",
});

itest!(_051_wasm_import {
args: "run --reload --allow-net --allow-read 051_wasm_import.ts",
output: "051_wasm_import.ts.out",
Expand Down