From 282b175e9402b7270a00fa7d286fbed31e6c9b04 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 00:06:26 -0400 Subject: [PATCH 01/12] prototype op_hello --- op_hello/Cargo.toml | 13 +++++++++++++ op_hello/src/hello.ts | 5 +++++ op_hello/src/lib.rs | 24 ++++++++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 op_hello/Cargo.toml create mode 100644 op_hello/src/hello.ts create mode 100644 op_hello/src/lib.rs diff --git a/op_hello/Cargo.toml b/op_hello/Cargo.toml new file mode 100644 index 00000000000000..68313b5e03ace6 --- /dev/null +++ b/op_hello/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "op_hello" +version = "0.1.0" +authors = ["Ryan Dahl "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +deno = "0.19.0" + +[dev-dependencies] +deno_typescript = "0.19.0" diff --git a/op_hello/src/hello.ts b/op_hello/src/hello.ts new file mode 100644 index 00000000000000..8269506165cabc --- /dev/null +++ b/op_hello/src/hello.ts @@ -0,0 +1,5 @@ +const OP_HELLO = Deno.ops.add("hello"); + +export function hello() { + Deno.send(OP_HELLO); +} diff --git a/op_hello/src/lib.rs b/op_hello/src/lib.rs new file mode 100644 index 00000000000000..12fd6e5da0561a --- /dev/null +++ b/op_hello/src/lib.rs @@ -0,0 +1,24 @@ +extern crate deno; +use deno::*; + +pub fn init(&mut isolate: Isolate) { + isolate.register_op("hello", op_hello); +} + +fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { + println!("Hello world"); + CoreOp::Sync(Box::new([])) +} + +#[test] +fn basic_test() { + match op_hello() { + CoreOp::Sync(buf) => { + assert_eq!(buf.len(), 0); + } + CoreOp::Async(_) => { + unreachable!() + } + } +} + From 404892cf5e64a2444c89816e63226aaba54d03db Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 00:18:31 -0400 Subject: [PATCH 02/12] WIP --- op_hello/src/hello_test.ts | 10 ++++++++++ op_hello/src/lib.rs | 16 +++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 op_hello/src/hello_test.ts diff --git a/op_hello/src/hello_test.ts b/op_hello/src/hello_test.ts new file mode 100644 index 00000000000000..8b393fa25077f3 --- /dev/null +++ b/op_hello/src/hello_test.ts @@ -0,0 +1,10 @@ +// TODO here we're referencing files accross crate boundaries, which has two +// problems: +// 1. "cargo package" breaks when you do this. +// 2. Using this crate outside of the deno tree becomes impossible. +import { test, assert } from "./src/js/test_util.ts"; +import { hello } from "./hello.ts"; + +test(function testHello(): void { + hello(); +}); diff --git a/op_hello/src/lib.rs b/op_hello/src/lib.rs index 12fd6e5da0561a..d87b98a29e052c 100644 --- a/op_hello/src/lib.rs +++ b/op_hello/src/lib.rs @@ -1,8 +1,11 @@ extern crate deno; use deno::*; -pub fn init(&mut isolate: Isolate) { +pub fn init(&mut isolate: Isolate) -> Result<(), ErrBox> { isolate.register_op("hello", op_hello); + + // TODO Somehow define register_deno_global + isolate.register_deno_global("src/hello.ts", "hello") } fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { @@ -11,14 +14,17 @@ fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { } #[test] -fn basic_test() { +fn hello_rust() { match op_hello() { CoreOp::Sync(buf) => { assert_eq!(buf.len(), 0); } - CoreOp::Async(_) => { - unreachable!() - } + CoreOp::Async(_) => unreachable!(), } } +#[test] +fn hello_js() { + // TODO need to define run_js_test somehow... + run_js_test("src/hello_test.ts"); +} From 2c203d7addd542d102531d001cae321bd26ce8d5 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 00:25:08 -0400 Subject: [PATCH 03/12] wip --- op_hello/src/hello_test.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/op_hello/src/hello_test.ts b/op_hello/src/hello_test.ts index 8b393fa25077f3..dc422edfaceaee 100644 --- a/op_hello/src/hello_test.ts +++ b/op_hello/src/hello_test.ts @@ -1,8 +1,18 @@ -// TODO here we're referencing files accross crate boundaries, which has two -// problems: -// 1. "cargo package" breaks when you do this. -// 2. Using this crate outside of the deno tree becomes impossible. -import { test, assert } from "./src/js/test_util.ts"; +// TODO We need some way to import test modules. +// Attempt one: +// +// import { test, assert } from "../../js/test_util.ts"; +// +// Here it is referencing files across crate boundaries, which will break +// 'cargo package' and means the crate is not useable outside the deno tree. +// This might be okay for a first pass, but it's not the best solution. +// +// Attempt two: +// we invent a new URL for referencing files in other crates. +// this is magic and not browser compatible.. Browser compatibility for +// ops is not so important. +import { test, assert } from "crate://deno_cli_snapshots@0.19.0/test_util.ts"; + import { hello } from "./hello.ts"; test(function testHello(): void { From ce4ae67cc5b739250b3fd190d84a8a2e84e91a2d Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 00:34:06 -0400 Subject: [PATCH 04/12] wip --- op_hello/Cargo.toml | 5 +---- op_hello/src/hello_test.ts | 7 +++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/op_hello/Cargo.toml b/op_hello/Cargo.toml index 68313b5e03ace6..0460193cb1dd81 100644 --- a/op_hello/Cargo.toml +++ b/op_hello/Cargo.toml @@ -4,10 +4,7 @@ version = "0.1.0" authors = ["Ryan Dahl "] edition = "2018" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - [dependencies] deno = "0.19.0" +deno_std = "0.19.0" # TODO deno_std is not yet published as crate -[dev-dependencies] -deno_typescript = "0.19.0" diff --git a/op_hello/src/hello_test.ts b/op_hello/src/hello_test.ts index dc422edfaceaee..71c2fe477379d3 100644 --- a/op_hello/src/hello_test.ts +++ b/op_hello/src/hello_test.ts @@ -1,7 +1,7 @@ // TODO We need some way to import test modules. // Attempt one: // -// import { test, assert } from "../../js/test_util.ts"; +// import { test } from "../../js/test_util.ts"; // // Here it is referencing files across crate boundaries, which will break // 'cargo package' and means the crate is not useable outside the deno tree. @@ -11,10 +11,9 @@ // we invent a new URL for referencing files in other crates. // this is magic and not browser compatible.. Browser compatibility for // ops is not so important. -import { test, assert } from "crate://deno_cli_snapshots@0.19.0/test_util.ts"; - +import { test } from "crate://deno_std@0.19.0/testing/mod.ts"; import { hello } from "./hello.ts"; -test(function testHello(): void { +test("hello test", () => { hello(); }); From 834a2b9feb808e69f345b1bbce52b294148dcef7 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 01:52:30 -0400 Subject: [PATCH 05/12] x --- op_hello/Cargo.toml | 11 ++++++++++- op_hello/src/hello.ts | 15 +++++++++++---- op_hello/src/hello_test.ts | 10 ++++++++-- op_hello/src/lib.rs | 20 +++++++++++++++----- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/op_hello/Cargo.toml b/op_hello/Cargo.toml index 0460193cb1dd81..61c6d2c0e22375 100644 --- a/op_hello/Cargo.toml +++ b/op_hello/Cargo.toml @@ -1,10 +1,19 @@ +# This is an imagined crate directory structure for Deno ops. + [package] name = "op_hello" version = "0.1.0" authors = ["Ryan Dahl "] edition = "2018" +# Note: I am explicitly attempting to not have a build.rs file. I think it might +# be unnecessary for each op crate to implement their own. Then again, it might +# be unavoidable... Let's try without first. + [dependencies] deno = "0.19.0" -deno_std = "0.19.0" # TODO deno_std is not yet published as crate +# deno_std is needed because hello_test.ts depends on the "testing/mod.ts" file +# inside of deno_std. +# TODO deno_std is not yet published as crate +deno_std = "0.19.0" diff --git a/op_hello/src/hello.ts b/op_hello/src/hello.ts index 8269506165cabc..840d7a3fccd38c 100644 --- a/op_hello/src/hello.ts +++ b/op_hello/src/hello.ts @@ -1,5 +1,12 @@ -const OP_HELLO = Deno.ops.add("hello"); +const OP_HELLO: number = Deno.ops.add("hello"); -export function hello() { - Deno.send(OP_HELLO); -} +/** + * The typedoc here ideally would be presevered automatically in + * lib.deno_runtime.d.ts + * Potentially by using this new method of defining functions on Deno, the d.ts + * file generated by typescript will be nicer. If not, we will keep maintaining + * lib.deno_runtime.d.ts by hand until we automate a solution. + */ +Deno.hello = () => { + Deno.core.send(OP_HELLO); +}; diff --git a/op_hello/src/hello_test.ts b/op_hello/src/hello_test.ts index 71c2fe477379d3..a3ee3b5186c126 100644 --- a/op_hello/src/hello_test.ts +++ b/op_hello/src/hello_test.ts @@ -1,4 +1,4 @@ -// TODO We need some way to import test modules. +// We need some way to import test modules. // Attempt one: // // import { test } from "../../js/test_util.ts"; @@ -11,7 +11,13 @@ // we invent a new URL for referencing files in other crates. // this is magic and not browser compatible.. Browser compatibility for // ops is not so important. -import { test } from "crate://deno_std@0.19.0/testing/mod.ts"; +// +// import { test } from "crate://deno_std@0.19.0/testing/mod.ts"; +// +// This is quite nice. But the version of deno_std already specified in +// Cargo.toml. I think we shouldn't repeat it. +import { test } from "crate://deno_std/testing/mod.ts"; + import { hello } from "./hello.ts"; test("hello test", () => { diff --git a/op_hello/src/lib.rs b/op_hello/src/lib.rs index d87b98a29e052c..3eb873e178fbaf 100644 --- a/op_hello/src/lib.rs +++ b/op_hello/src/lib.rs @@ -1,11 +1,22 @@ extern crate deno; +extern crate deno_std; use deno::*; pub fn init(&mut isolate: Isolate) -> Result<(), ErrBox> { - isolate.register_op("hello", op_hello); + isolate.register_op("hello", op_hello); // register_op defined by #3002 - // TODO Somehow define register_deno_global - isolate.register_deno_global("src/hello.ts", "hello") + // Explicitly link the deno_std crate so it can be used in hello_test.ts + // Its usage looks like this: + // + // import { test } from "crate://deno_std/testing/mod.ts"; + // + // In the future it might make sense to automate this function away, but I think + // it would be prudent to make the crate URL resolution as obvious as + // possible. + isolate.register_crate_url("deno_std", deno_std::get_file); + + // TODO The ability to run typescript doesn't exist in deno core. + isolate.run("src/hello.ts") } fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { @@ -25,6 +36,5 @@ fn hello_rust() { #[test] fn hello_js() { - // TODO need to define run_js_test somehow... - run_js_test("src/hello_test.ts"); + deno_test("src/hello_test.ts"); // TODO implement deno_test } From 775f56a2060b7f0e5de0a756bb32808d36c973bd Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 01:55:10 -0400 Subject: [PATCH 06/12] x --- op_hello/src/hello_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/op_hello/src/hello_test.ts b/op_hello/src/hello_test.ts index a3ee3b5186c126..75566fa9978e75 100644 --- a/op_hello/src/hello_test.ts +++ b/op_hello/src/hello_test.ts @@ -18,8 +18,8 @@ // Cargo.toml. I think we shouldn't repeat it. import { test } from "crate://deno_std/testing/mod.ts"; -import { hello } from "./hello.ts"; +import "./hello.ts"; test("hello test", () => { - hello(); + Deno.hello(); }); From e9e0e80e154998c7df9da278498b13e2d3a882fd Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 02:25:32 -0400 Subject: [PATCH 07/12] Maybe we can use split file namespaces --- op_hello/src/hello.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/op_hello/src/hello.ts b/op_hello/src/hello.ts index 840d7a3fccd38c..ab4ca02f69a218 100644 --- a/op_hello/src/hello.ts +++ b/op_hello/src/hello.ts @@ -1,12 +1,12 @@ const OP_HELLO: number = Deno.ops.add("hello"); -/** - * The typedoc here ideally would be presevered automatically in - * lib.deno_runtime.d.ts - * Potentially by using this new method of defining functions on Deno, the d.ts - * file generated by typescript will be nicer. If not, we will keep maintaining - * lib.deno_runtime.d.ts by hand until we automate a solution. - */ -Deno.hello = () => { - Deno.core.send(OP_HELLO); -}; +// https://www.typescriptlang.org/docs/handbook/namespaces.html#splitting-across-files +namespace Deno { + /** + * The typedoc here ideally would be presevered automatically in + * lib.deno_runtime.d.ts + */ + export function hello() { + Deno.core.send(OP_HELLO); + } +} From 21aa00a0a22eb4fd41cc023467221e0670d04ac7 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 02:45:35 -0400 Subject: [PATCH 08/12] elegant url --- op_hello/src/hello_test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/op_hello/src/hello_test.ts b/op_hello/src/hello_test.ts index 75566fa9978e75..913568f73c6599 100644 --- a/op_hello/src/hello_test.ts +++ b/op_hello/src/hello_test.ts @@ -18,8 +18,17 @@ // Cargo.toml. I think we shouldn't repeat it. import { test } from "crate://deno_std/testing/mod.ts"; +// If we don't do the //src reorg that I've proposed in #3022, then we might be +// able to have a very elegant URL some day using the deno crate. +// +// import { test } from "crate://deno/std/testing/mod.ts"; + import "./hello.ts"; test("hello test", () => { Deno.hello(); }); + +test("hello test2", () => { + Deno.hello(); +}); From d8c20c0ec66953b2f8b4a8bd22ac3da3b2c7ef3b Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 02:50:46 -0400 Subject: [PATCH 09/12] x --- op_hello/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/op_hello/src/lib.rs b/op_hello/src/lib.rs index 3eb873e178fbaf..5d4ef2df8f9aeb 100644 --- a/op_hello/src/lib.rs +++ b/op_hello/src/lib.rs @@ -25,7 +25,7 @@ fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { } #[test] -fn hello_rust() { +fn rust_test() { match op_hello() { CoreOp::Sync(buf) => { assert_eq!(buf.len(), 0); @@ -35,6 +35,7 @@ fn hello_rust() { } #[test] -fn hello_js() { - deno_test("src/hello_test.ts"); // TODO implement deno_test +fn js_test() { + // This should execute src/hello_test.ts + deno_test(); } From 607aeaec29de4967a2dda57bd14fe628e0414dc5 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 03:09:31 -0400 Subject: [PATCH 10/12] Add op_hello_js --- op_hello_js/Cargo.toml | 15 +++++++++++++++ op_hello_js/src/hello.js | 5 +++++ op_hello_js/src/hello_test.js | 4 ++++ op_hello_js/src/lib.rs | 29 +++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+) create mode 100644 op_hello_js/Cargo.toml create mode 100644 op_hello_js/src/hello.js create mode 100644 op_hello_js/src/hello_test.js create mode 100644 op_hello_js/src/lib.rs diff --git a/op_hello_js/Cargo.toml b/op_hello_js/Cargo.toml new file mode 100644 index 00000000000000..80502df3ab90bf --- /dev/null +++ b/op_hello_js/Cargo.toml @@ -0,0 +1,15 @@ +# This is an imagined crate directory structure for Deno ops. +# Same as op_hello but done with JS and not TS. + +[package] +name = "op_hello_js" +version = "0.1.0" +authors = ["Ryan Dahl "] +edition = "2018" + +# Note: I am explicitly attempting to not have a build.rs file. I think it might +# be unnecessary for each op crate to implement their own. Then again, it might +# be unavoidable... Let's try without first. + +[dependencies] +deno = "0.19.0" diff --git a/op_hello_js/src/hello.js b/op_hello_js/src/hello.js new file mode 100644 index 00000000000000..896d1862ec6ff9 --- /dev/null +++ b/op_hello_js/src/hello.js @@ -0,0 +1,5 @@ +const OP_HELLO = Deno.ops.add("hello"); + +export function hello() { + Deno.core.send(OP_HELLO); +} diff --git a/op_hello_js/src/hello_test.js b/op_hello_js/src/hello_test.js new file mode 100644 index 00000000000000..5fd5d0c050319e --- /dev/null +++ b/op_hello_js/src/hello_test.js @@ -0,0 +1,4 @@ +// As opposed to the TypeScript op_hello example, here we want to not rely on +// deno_std, as it uses TypeScript. So here don't use a test runner. +import { hello } from "./hello.js"; +hello(); diff --git a/op_hello_js/src/lib.rs b/op_hello_js/src/lib.rs new file mode 100644 index 00000000000000..811da8004f5153 --- /dev/null +++ b/op_hello_js/src/lib.rs @@ -0,0 +1,29 @@ +extern crate deno; +extern crate deno_std; +use deno::*; + +pub fn init(&mut isolate: Isolate) -> Result<(), ErrBox> { + isolate.register_op("hello", op_hello); // register_op defined by #3002 + + isolate.execute("src/hello.js") +} + +fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { + println!("Hello world"); + CoreOp::Sync(Box::new([])) +} + +#[test] +fn rust_test() { + match op_hello() { + CoreOp::Sync(buf) => { + assert_eq!(buf.len(), 0); + } + CoreOp::Async(_) => unreachable!(), + } +} + +#[test] +fn js_test() { + isolate.execute("src/hello_test.js") +} From f6f169d8aa68ebc197532ce92b725f39c7f102ef Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 05:44:37 -0400 Subject: [PATCH 11/12] simplify --- op_hello_js/Cargo.toml | 5 ++--- op_hello_js/{src => }/hello.js | 0 op_hello_js/{src => }/hello_test.js | 0 op_hello_js/{src => }/lib.rs | 22 ++++++++++------------ 4 files changed, 12 insertions(+), 15 deletions(-) rename op_hello_js/{src => }/hello.js (100%) rename op_hello_js/{src => }/hello_test.js (100%) rename op_hello_js/{src => }/lib.rs (62%) diff --git a/op_hello_js/Cargo.toml b/op_hello_js/Cargo.toml index 80502df3ab90bf..5e6e44946ffa8b 100644 --- a/op_hello_js/Cargo.toml +++ b/op_hello_js/Cargo.toml @@ -7,9 +7,8 @@ version = "0.1.0" authors = ["Ryan Dahl "] edition = "2018" -# Note: I am explicitly attempting to not have a build.rs file. I think it might -# be unnecessary for each op crate to implement their own. Then again, it might -# be unavoidable... Let's try without first. +[lib] +path = "lib.rs" [dependencies] deno = "0.19.0" diff --git a/op_hello_js/src/hello.js b/op_hello_js/hello.js similarity index 100% rename from op_hello_js/src/hello.js rename to op_hello_js/hello.js diff --git a/op_hello_js/src/hello_test.js b/op_hello_js/hello_test.js similarity index 100% rename from op_hello_js/src/hello_test.js rename to op_hello_js/hello_test.js diff --git a/op_hello_js/src/lib.rs b/op_hello_js/lib.rs similarity index 62% rename from op_hello_js/src/lib.rs rename to op_hello_js/lib.rs index 811da8004f5153..ecd98b15a40694 100644 --- a/op_hello_js/src/lib.rs +++ b/op_hello_js/lib.rs @@ -1,11 +1,10 @@ extern crate deno; -extern crate deno_std; use deno::*; pub fn init(&mut isolate: Isolate) -> Result<(), ErrBox> { isolate.register_op("hello", op_hello); // register_op defined by #3002 - - isolate.execute("src/hello.js") + isolate.execute("hello.js")?; + Ok(()) } fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { @@ -14,16 +13,15 @@ fn op_hello(_control_buf: &[u8], _zero_copy_buf: Option) -> CoreOp { } #[test] -fn rust_test() { - match op_hello() { - CoreOp::Sync(buf) => { - assert_eq!(buf.len(), 0); - } - CoreOp::Async(_) => unreachable!(), - } +fn js_test() { + isolate.execute("hello_test.js") } #[test] -fn js_test() { - isolate.execute("src/hello_test.js") +fn rust_test() { + if let CoreOp::Sync(buf) = op_hello() { + assert_eq!(buf.len(), 0); + } else { + unreachable!(); + } } From b04616a1d17ffd4f8f4ff4540f56e942382e3a26 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 26 Sep 2019 05:52:27 -0400 Subject: [PATCH 12/12] adjust --- op_hello_js/hello.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/op_hello_js/hello.js b/op_hello_js/hello.js index 896d1862ec6ff9..7cfdaa82b871f3 100644 --- a/op_hello_js/hello.js +++ b/op_hello_js/hello.js @@ -1,5 +1,6 @@ -const OP_HELLO = Deno.ops.add("hello"); +// TODO In the future maybe we can extract the op id in the top-level and use a +// constant. But currently it's causing problems with snapshotting. export function hello() { - Deno.core.send(OP_HELLO); + Deno.core.send(Deno.ops["hello"]); }