Questions, problems and documentation requests

[ebebbington]

Readme suggestions

1. The --unstable flag is needed in the usage section to run deno2node
2. "create corresponding deps.node.ts." could be renamed to "and copy the contents of the deno.deps.ts file to a new node.deps.ts file" - it isn't entirely understandable that the node.deps.ts file needs to be the same (or does it?)
3. If your project doesnt have deno globals, you wouldnt pass in a tsconfig.json arg, yet the docs tell you to regardless of any scenario. Also, you have to pass one in, which should be optional (as in some projects, they might not use the deno namespace)

Suggestions

4. deno2node also tries transpiling certain files you dont want it to, such as test files, there shoudl be an ignore option
5. A nice example of what a shim.node.ts should look like would be nice, i was scratching my head for a while until i decided to check your source code to see if you had one and how that looked
6. lack of test files is worrying
7. Why was a tsconfig.json chosen to specify the shim? surely that file shouldn't be responsible for holding these configurations (and goes against the schema right?)
8. Be nice to restrict perms even more, eg the docs specify --allow-write=src --allow-net=deno.land

[wojpawlik]

tsconfig.json is required by CLI and used to configure

• deno2node.shim,
• files, or include and exclude,
• compilerOptions, except https://github.com/wojpawlik/deno2node/blob/d224750108fcdc70d666b3cc3f9e95e6310a4a47/src/context.ts#L4-L5

1. The --unstable flag is needed in the usage section

Right, I'll add it in next commit. --no-check used to suffice.

6. lack of test files is worrying

The tool is end-to-end tested: https://github.com/wojpawlik/deno2node/blob/d224750108fcdc70d666b3cc3f9e95e6310a4a47/.github/workflows/ci.yml#L50-L59

<== To Be Continued ===

[wojpawlik]

2. "create corresponding deps.node.ts." could be renamed to "and copy the contents of the deno.deps.ts file to a new node.deps.ts file" - it isn't entirely understandable that the node.deps.ts file needs to be the same (or does it?)

*.deno.ts and *.node.ts files are for runtime-specific code. They should be different, that's the point!

8. Be nice to restrict perms even more, eg the docs specify --allow-write=src

I'd rather not. --allow-read='.' assumes that cwd is project root.

5. A nice example of what a shim.node.ts should look like would be nice, i was scratching my head for a while until i decided to check your source code

README.md and API reference already link to my shim.node.ts.

Still, I'll try to rewrite README.md to include a more comprehensive shim.node.ts, and all the information I shared here.

[wojpawlik]

*.deno.ts and *.node.ts files are surprisingly difficult to explain for something implemented in just 5 lines https://github.com/wojpawlik/deno2node/blob/5b4f96f1efd91fd81cdd40548dd4520a6b59ac0c/src/_transformations/specifiers.ts#L27 https://github.com/wojpawlik/deno2node/blob/5b4f96f1efd91fd81cdd40548dd4520a6b59ac0c/src/deno2node.ts#L72-L75

[KnorpelSenf]

surprisingly difficult to explain

I believe the easiest way to do this is to show example code. For instance, write a small program that counts the characters in a text file, and prints it to stdout (i.e. wc -m). The logic is trivial, and everyone can understand it. The only call that is platform-specific is reading a text file (sync for simplicity), so it goes into the .node.ts and .deno.ts files.

[wojpawlik]

Not a great example. It can be solved more easily with just

import { lstatSync } from "https://deno.land/std@0.101.0/node/fs.ts";

or

import { readFileSync } from "https://deno.land/std@0.101.0/node/fs.ts";

or

// @filename: src/shim.node.ts
export * from "deno.ns";

I'll release 0.7.1 as-is and leave the issue open until runtime-specific files are explained in Readme.

[KnorpelSenf]

I think we're suggesting the exact same thing. Either way, using an example in the explanation seems to be something we agree on 👍🏻

[wojpawlik]

Should I replace --allow-write with --allow-write=<outDir> in README.md?