denoland/deno

basic node compatibity mode

ry opened this issue · 21 comments

ry commented

Make the following work:

mkdir deno-project && cd deno-project && yarn init deno-project -y
yarn add rollup
echo 'import { VERSION } from "rollup";' > foo.mjs
echo 'console.log(VERSION);' >> foo.mjs
deno run --compat foo.mjs
  • built-in node modules #12293
  • "node:" prefixes for built-in modules #12337
  • inject and execute "https://deno.land/std/node/global.ts" to setup "process" and friends #12342
  • Figure out X-Deno-Warning coming from Node built-in mappings #12396
  • Add docs to manual for built-in node modules. denoland/manual#105
  • npm-resolution. Roughly: look in the current directory for node_modules/ folder (actually walk to root from cwd looking for them), open $bare_specifier/package.json, load file corresponding to "module" entry #12424
  • LSP support

Hey,
tsserver has annoyed me to a point where I'd do anything to have a better/faster alternative, including tackling this issue ;-)

Is there currently any work being done on this, or has someone else already picked up this issue and could give me some pointers on where I could help? Simply resolving packages from the local node_modules folders seems pretty straightforward to me, even though this just covers packages with an explicit module field.
Down the road it would also be interesting to look into handling yarn v2 import since it also uses a central package folder instead of local node_modules (which is similar to what deno does afaik?).

Compatibility mode should be enabled per-import, so that users aren't forced to "poison" entire process for a single Node-only dependency.


CJS and extensionless imports are on the way out: https://blog.sindresorhus.com/hello-modules-d1010b4e777b. https://deno.land/std/node/import_map.json would simplify writing or generating ESM that works on both runtimes.

Compatibility mode should be enabled per-import, so that users aren't forced to "poison" entire process for a single Node-only dependency.

That sounds nice, but is not really feasible to apply different resolution schemes per import.

CJS and extensionless imports are on the way out: https://blog.sindresorhus.com/hello-modules-d1010b4e777b.

I don't believe that's true. It will take years before CJS are abandoned by community and few more to be removed from Node (if ever).

kt3k commented

load file corresponding to "module" entry

Do we only use "module" property? I believe "module" property is a non-standard property used by some bundlers like rollup, webpack, etc, and most npm packages don't provide it.

Another question: Do we provide global require function?

Could the flag name be changed from --compat to anything else? IMO it's not descriptive and it should better reflect that it's a compatibility mode with node.js.

--nodejs-compat?

Thinking about:

Figure out X-Deno-Warning coming from Node built-in mappings

I'm leaning towards bundling std/node in the binary. We already have deno_std as a submodule in the tree, so it should be trivial to bundle that code during build. It would get solve the versioning problem (of using trunk version) and instead it would used pinned version. Unfortunately it means we'd have to keep deno_std very much up-to-date in the tree and users if we wanted users to have the latest version; even then it would be at least one version behind latest "deno_std".

Curious to hear other ideas.

hayd commented

optionally passing the std version (with a recent default that could be overridden), would at least enable node/std to update independently of deno.

e.g. --compat be equivalent to --compat=0.110.0 etc. 😬
edit: or perhaps using the url might be preferable --compat='https://deno.land/std@0.110.0/node/'
(that way, for example, users could test fixes to std node, or roll their own for whatever reason.)

update: #12508 makes this an env variable

vwkd commented

Unfortunately it means we'd have to keep deno_std very much up-to-date in the tree and users if we wanted users to have the latest version; even then it would be at least one version behind latest "deno_std".

Does it need to be one version behind? Why not release deno_std before deno, update the deno tree, then release deno...?

Figure out X-Deno-Warning coming from Node built-in mappings

I'm leaning towards bundling std/node in the binary.

Why not simply pin a deno_std version on CLI release instead of always requesting latest?

Unfortunately it means we'd have to keep deno_std very much up-to-date in the tree and users if we wanted users to have the latest version; even then it would be at least one version behind latest "deno_std".

Does it need to be one version behind? Why not release deno_std before deno, update the deno tree, then release deno...?

That's the current release workflow, additionally an update to dotland repo needs to be merged before CLI and std are available for discovery.

Figure out X-Deno-Warning coming from Node built-in mappings

I'm leaning towards bundling std/node in the binary.

Why not simply pin a deno_std version on CLI release instead of always requesting latest?

Because it would have to be one version behind as mentioned above.

kt3k commented

I think we should keep Object.prototype.__proto__ in compat mode. In my previous experiment with std/node, I found some heavily dependent npm modules (such as chalk, acorn-node, etc) depend on that feature. Keeping __proto__ should improve the compatibility significantly

I think we should keep Object.prototype.__proto__ in compat mode. In my previous experiment with std/node, I found some heavily dependent npm modules (such as chalk, acorn-node, etc) depend on that feature. Keeping __proto__ should improve the compatibility significantly

I'm not a fan of that, we'd need to branch our "runtime setup" code to achieve that. @kitsonk do you have an opinion?

It is a very bad idea. The number of security advisories that are related to __proto__. It has been dropped from the spec. I would be curious to know why current versions of chalk and acorn require access and what we can do to support them without reintroducing a security issue, even in a compat mode.

It is a very bad idea. The number of security advisories that are related to __proto__. It has been dropped from the spec. I would be curious to know why current versions of chalk and acorn require access and what we can do to support them without reintroducing a security issue, even in a compat mode.

It hasn't been dropped from the spec, it was moved out of annex B and made normative optional: tc39/ecma262#2125

It is a very bad idea. The number of security advisories that are related to __proto__. It has been dropped from the spec. I would be curious to know why current versions of chalk and acorn require access and what we can do to support them without reintroducing a security issue, even in a compat mode.

I checked chalk and couldn't find a single reference to it, as well as its dependencies. I suggest we punt on it, and reevaluate if it becomes a huge pain point; I'm skeptical anyone will hit it in the first weeks after introducing of --compat.

It is a very bad idea. The number of security advisories that are related to __proto__. It has been dropped from the spec. I would be curious to know why current versions of chalk and acorn require access and what we can do to support them without reintroducing a security issue, even in a compat mode.

It hasn't been dropped from the spec, it was moved out of annex B and made normative optional: tc39/ecma262#2125

Apologies... I just remember the debate, forgot it was moved to Annex B. I got confused on the effort to remove everything from Annex B, but that it was a compromise to move this to Annex B. Either way, it shouldn't be there and shouldn't be used.

How about adding another flag that can only be used in compat mode, like --compat --enable-proto?

NOTE: Node.js has a --disable-proto=mode option

ije commented

Can we change the user-agent for remote modules too when the --compat flag passed?for example Deno/1.15.1 (compat), with this the CDN (esm.sh/sykpack) can have better compatibility!

ry commented

Can we change the user-agent for remote modules too when the --compat flag passed?for example Deno/1.15.1 (compat), with this the CDN (esm.sh/sykpack) can have better compatibility!

That's an interesting idea. I think we should let compat mode settle down a bit first. We're still actively working on it. Ideally compat mode would not require any changes on the cdns.

ry commented

Basic compat mode is working. Closing this issue and moving to #12577