cli/js/ directory restructure

[bartlomieju]

It's high time to take another look at directory structure of cli/js/. For now we've strived to keep it as flat as possible, but in the long run there are a few problems with that approach.

cli/js/ contains both runtime code as well as tests for that code. Runtime code is bundled and snapshotted during build step using deno_typescript. Runtime code is written as ESModules but as @kitsonk and I have found several times it's not really valid ESModules (performs global scope augmentation as a side effect) and really convoluted. Additionally runtime code should be entirely self-contained; it shouldn't really on any imports outside cli/js/.

Presence of *_test.ts files alongside runtime code is very misleading - all cli/js/*_test.ts files are valid programs that can be (and are) run using deno run command. Contrary to runtime code, tests can import third-party code (eg. imports utilities from std/ directory). Additionally test code relies heavily on cli/js/unit_test_runner.ts which is yet another Deno program that implements some specific logic to testing runtime code (handling test cases with different permission requirements). I'm actively working towards testing API that would allow to get rid of that bespoke
runner, but it is to stay for another weeks.

Then we arrive to intricate problem of required ops for different workers (main, web worker, compiler). Currently some workers have access to ops they should not really depend on, but ops code is structured in such a way that prevents fixing this problem. This problem can be boiled down to: runtime code is written in such a way that it's not possible to include single op from a group of related ops. It's very much connected to #2180.

My proposition and rationale for each point:

a) Reorganise cli/js/ file structure to better resemble cli/ops structure where ops are grouped like in Rust. This step will make it obvious which files implement ops and in turn which files consume ops. It is also a prerequisite to making each op a standalone thing.

b) Place code implementing Web APIs in a single directory. Some of this code depends on ops (eg. fetch) therefore it's a good idea clearly see which files are files are pure JS and which consume ops.

c) Move main runtime setup code into a single file. At the moment cli/js/runtime_main.ts imports a bunch of function that set different global properties (eg. window.location). All mutations of global variables should be centralised into runtime_main.ts.

d) Move runtime test code to a separate directory. I'm not really sure where exactly this code should live, but moving it to separate location appears to be a good idea to point out that testing files are valid Deno programs. Ultimately we should strive to make test each file self-contained to allow making each op a standalone thing (it won't be possible at the moment).

CC
@ry
@kitsonk for your opinion regarding runtime code

[nayeemrmn]

d) Move runtime test code to a separate directory.

It's true it was confusing at first that cli/js/*_test.ts were userland modules unlike their subjects, but it's still practically convenient to have them nearby.

[ry]

a) Reorganise cli/js/ file structure to better resemble cli/ops structure where ops are grouped like in Rust. This step will make it obvious which files implement ops and in turn which files consume ops. It is also a prerequisite to making each op a standalone thing.

I agree, the cli/ops/ structure makes a lot more sense than the one in cli/js/.
I think the question is just which of the following do we use (using fs ops as an example) :

• cli/js/ops/fs.ts
• cli/ops/fs.ts
• cli/js/fs.ts

b) Place code implementing Web APIs in a single directory. Some of this code depends on ops (eg. fetch) therefore it's a good idea clearly see which files are files are pure JS and which consume ops.

Agree. So cli/js/web/ ?

c) Move main runtime setup code into a single file. At the moment cli/js/runtime_main.ts imports a bunch of function that set different global properties (eg. window.location). All mutations of global variables should be centralised into runtime_main.ts.

Agree.

d) Move runtime test code to a separate directory. I'm not really sure where exactly this code should live, but moving it to separate location appears to be a good idea to point out that testing files are valid Deno programs. Ultimately we should strive to make test each file self-contained to allow making each op a standalone thing (it won't be possible at the moment).

Agree - with its own readme explaining that the tests run under the Deno runtime as opposed to being bundled & snapshotted via deno_typescript. cli/js/tests is my first instinct.

I'd also like to get rid of this random cli/js/mixins directory. It's not special enough to have its own directory.

[bartlomieju]

a) Reorganise cli/js/ file structure to better resemble cli/ops structure where ops are grouped like in Rust. This step will make it obvious which files implement ops and in turn which files consume ops. It is also a prerequisite to making each op a standalone thing.

I agree, the cli/ops/ structure makes a lot more sense than the one in cli/js/.
I think the question is just which of the following do we use (using fs ops as an example) :

• cli/js/ops/fs.ts
• cli/ops/fs.ts
• cli/js/fs.ts

I'd opt for first option; doesn't move TS files outside cli/js/ directory and should be fairly easy to break out ops into separate directories.

b) Place code implementing Web APIs in a single directory. Some of this code depends on ops (eg. fetch) therefore it's a good idea clearly see which files are files are pure JS and which consume ops.

Agree. So cli/js/web/ ?

Yes.

c) Move main runtime setup code into a single file. At the moment cli/js/runtime_main.ts imports a bunch of function that set different global properties (eg. window.location). All mutations of global variables should be centralised into runtime_main.ts.

Agree.

d) Move runtime test code to a separate directory. I'm not really sure where exactly this code should live, but moving it to separate location appears to be a good idea to point out that testing files are valid Deno programs. Ultimately we should strive to make test each file self-contained to allow making each op a standalone thing (it won't be possible at the moment).

Agree - with its own readme explaining that the tests run under the Deno runtime as opposed to being bundled & snapshotted via deno_typescript. cli/js/tests is my first instinct.

I'd also like to get rid of this random cli/js/mixins directory. It's not special enough to have its own directory.

👍 will do

[bartlomieju]

d) Move runtime test code to a separate directory.

It's true it was confusing at first that cli/js/*_test.ts were userland modules unlike their subjects, but it's still practically convenient to have them nearby.

@nayeemrmn I agree with your point. Ultimately we want to arrive at a situation where each op is self-contained with it's tests; something along the lines of:

ops/fs/chmod/
    mod.rs
    mod.ts
    test.ts

[kitsonk]

Just because I was asked, I am for anything that is somewhat sane! It tends to be a situation where the less cooks in the kitchen it is easier, so my opinion is do something logical and sane and I will be happy with it!

[kevinkassimo]

ops/fs/chmod/
    mod.rs
    mod.ts
    test.ts

+1 for this structure. There could also be a mod.rs on the level of ops/fs to do the actual op registering.

[bartlomieju]

With all of above PRs landed I deem this issue can be closed. Next step would be to actually factor out ops into separate crates/modules.