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

Improve Node Compat Mode #12577

Closed
13 of 18 tasks
ry opened this issue Oct 28, 2021 · 35 comments
Closed
13 of 18 tasks

Improve Node Compat Mode #12577

ry opened this issue Oct 28, 2021 · 35 comments
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted) node compat

Comments

@ry
Copy link
Member

ry commented Oct 28, 2021

Make various popular projects run:

Features:

@ry ry mentioned this issue Oct 28, 2021
7 tasks
@bartlomieju
Copy link
Member

Next.js is currently blocked on missing support for IPC for subprocesses (ChildProcess.send API; https://nodejs.org/api/child_process.html#subprocesssendmessage-sendhandle-options-callback)

@piscisaureus
Copy link
Member

@bartlomieju

Next.js is currently blocked on missing support for IPC for subprocesses (ChildProcess.send API; https://nodejs.org/api/child_process.html#subprocesssendmessage-sendhandle-options-callback)

Is it using the "sendHandle" feature of IPC channels, or does it send only JSON messages?

@bartlomieju
Copy link
Member

@piscisaureus it looks like only this signature is used:

send(message: Serializable, callback?: (error: Error | null) => void): boolean;

@bartlomieju
Copy link
Member

ESLint is now working in compat mode:

Screenshot_2021-10-31_at_12 24 51

Screenshot_2021-10-31_at_12 37 30

@bartlomieju
Copy link
Member

Parcel uses "native addons" (.node files), so I'm not sure we'll be able to run that at all.

@banocean
Copy link

banocean commented Nov 1, 2021

https://nodejs.org/api/tls.html for mongodb

@lucacasonato lucacasonato added feat new feature (which has been agreed to/accepted) cli related to cli/ dir node compat labels Nov 3, 2021
@ry ry pinned this issue Nov 7, 2021
@Jack-Works
Copy link

Jack-Works commented Nov 23, 2021

I'm tring to run webpack in deno.

Problems and solutions:

copy-webpack-plugin uses setTimeout().unref() (a NodeJS feature)

Solution: patch setTimeout and clearTimeout.

copy-webpack-plugin uses dynamic import syntax to import package globby and the module resolution comes back to Deno and leads to error.

Solution: use an import map to redirect the import

globby is a Node ESM only package

Impossible to import it in Deno because it's not using require.

Solution: I manually compile globby into CommonJS.

https is not defined

Solution: N/A

swc-loader is loading @swc/core which is a Node native extension (.node file)

Solution: I write my own loader that uses https://deno.land/x/swc@0.1.4 (in WASM) but its version of SWC is a little bit old.

There're some other mysterious problems in the build and I didn't investigate them yet.

Conclusion

I only tried calling webpack programmatically, generally, it works well.

There're some problems with copy-webpack-plugin and html-webpack-plugin, part of them can be fixed by patching some modules.

I only tried calling webpack by

webpack(createConfig(), callback)

I didn't try:

  • Webpack CLI
  • Webpack dev server

I think Deno's Node compat mode is doing very well, the other problems I encountered are not possible to fix (e.g. Node-style native ES Module).

Code URL

DimensionDev/Maskbook@cc8f6d2

@bartlomieju
Copy link
Member

Thanks for the info @Jack-Works

copy-webpack-plugin uses setTimeout().unref() (a NodeJS feature)
Solution: patch setTimeout and clearTimeout.

This is tracked in denoland/std#1622

copy-webpack-plugin uses dynamic import syntax to import package globby and the module resolution comes back to Deno and leads to error.
Solution: use an import map to redirect the import

This should work without a problem, Deno support dynamic imports and should resolve the package properly, can you point me to the relevant part in your branch?

globby is a Node ESM only package
Impossible to import it in Deno because it's not using require.

Solution: I manually compile globby into CommonJS.

In compat mode require is available in global, so again this should work (unless you're hitting #12648). Please point me to the relevant place in your code.

https is not defined
Solution: N/A

This is being worked on.

swc-loader is loading @swc/core which is a Node native extension (.node file)
Solution: I write my own loader that uses https://deno.land/x/swc@0.1.4 (in WASM) but its version of SWC is a little bit old.

Good solution, there's nothing we can do with .node files.

@Jack-Works

This comment has been minimized.

@bartlomieju
Copy link
Member

copy-webpack-plugin uses dynamic import syntax to import package globby and the module resolution comes back to Deno and leads to error.
Solution: use an import map to redirect the import

This should work without a problem, Deno support dynamic imports and should resolve the package properly, can you point me to the relevant part in your branch?

I have created a minimal reproduce at https://github.com/Jack-Works/deno-compat-test

I understand if deno decides not to support them because having two ES module resolutions in a single environment is a nightmare.

The real-world problem: globby is a pure Node-ESM module, therefore copy-webpack-plugin has to import it by dynamic import (also, in Node-ESM style). That requires a subtree of the module graph using a slightly different module resolution (e.g. Node package self-reference / node_module lookup).

This resolution algorithm is already supported by Deno in the compat mode. Thank you for reproduction, I will take a look and investigate, probably some kind of bug in the implementation.

@Jack-Works

This comment has been minimized.

@bartlomieju
Copy link
Member

This resolution algorithm is already supported by Deno in the compat mode. Thank you for reproduction, I will take a look and investigate, probably some kind of bug in the implementation.

Isn't that algorithm only be used when the module is loaded by require?

No, in compat mode, both CJS and ESM resolution works as in Node.js, see https://deno.land/manual@main/npm_nodejs/compatibility_mode#how-does-it-work

How is Deno going to use Node resolution for import?

We implemented a separate ESM resolver that follows Node.js algorithm: https://github.com/denoland/deno/blob/main/cli/compat/esm_resolver.rs

@Jack-Works
Copy link

No, in compat mode, both CJS and ESM resolution works as in Node.js, see https://deno.land/manual@main/npm_nodejs/compatibility_mode#how-does-it-work

Oh, that's cool. Is it possible to specify a Worker is in the compat mode? Because my goal is to compile webpack in a Worker and use the main thread to schedule multiple tasks.

@bartlomieju
Copy link
Member

No, in compat mode, both CJS and ESM resolution works as in Node.js, see https://deno.land/manual@main/npm_nodejs/compatibility_mode#how-does-it-work

Oh, that's cool. Is it possible to specify a Worker is in the compat mode? Because my goal is to compile webpack in a Worker and use the main thread to schedule multiple tasks.

No, not yet. There's denoland/std#1151 for that.

@Jack-Works
Copy link

No, in compat mode, both CJS and ESM resolution works as in Node.js, see https://deno.land/manual@main/npm_nodejs/compatibility_mode#how-does-it-work

Oh, that's cool. Is it possible to specify a Worker is in the compat mode? Because my goal is to compile webpack in a Worker and use the main thread to schedule multiple tasks.

No, not yet. There's denoland/deno_std#1151 for that.

I also need to specify a smaller range of security permissions (e.g. No network access) in the worker. I can do that in the Worker but is that possible in worker_threads?

@bartlomieju
Copy link
Member

No, in compat mode, both CJS and ESM resolution works as in Node.js, see https://deno.land/manual@main/npm_nodejs/compatibility_mode#how-does-it-work

Oh, that's cool. Is it possible to specify a Worker is in the compat mode? Because my goal is to compile webpack in a Worker and use the main thread to schedule multiple tasks.

No, not yet. There's denoland/deno_std#1151 for that.

I also need to specify a smaller range of security permissions (e.g. No network access) in the worker. I can do that in the Worker but is that possible in worker_threads?

No, I suggest to open a new issue in deno_std with your feature request, or just keep using Worker constructor.

@bartlomieju
Copy link
Member

@piscisaureus it looks like only this signature is used:

send(message: Serializable, callback?: (error: Error | null) => void): boolean;

I confirmed that next.js sends only serializable message. No handles are sent.

@Soremwar
Copy link
Contributor

#13227 should probably be included here

@kt3k
Copy link
Member

kt3k commented Feb 24, 2022

Ran unit tests of ESLint using the below command in eslint repository (I stripped nyc part of the command because it doesn't work with compat mode):

node ./node_modules/mocha/bin/_mocha  --forbid-only -R progress -t 10000 -c "tests/{bin,conf,lib,tools}/**/*.js"

with node is a wrapper shell script:

#!/bin/sh
DENO_NODE_COMPAT_URL=file:///path/to/denoland/deno_std/ deno run --no-check --unstable --compat -A $@

I got this result

  30855 passing (22m)
  534 failing

I haven't analyzed these 534 errors yet, but at least 98.3% of test cases passed.

@JLarky
Copy link

JLarky commented May 30, 2022

To help any unlucky soul who will try to come up with a way to test any of these claims :) I want to present to you a bit of a document on the actual state of Vite compatibility (i.e. running Vite with deno --compat).

How to start testing (requires node/yarn, also huge thanks to Ryan on the starting point with rollup):

mkdir deno-project && cd deno-project && yarn init deno-project -y
yarn add vite @vitejs/plugin-react @types/node
date > ./index.html
deno run --no-check --unstable --compat -A ./node_modules/.bin/vite

You will see


  vite v2.9.9 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  ready in 118ms.

Not implemented: ChildProcess.unref()

Build command doesn't work even in this simple case.

$ deno run --no-check --unstable --compat -A ./node_modules/.bin/vite build
vite v2.9.9 building for production...
Not implemented: process.on("beforeExit")
[vite:reporter] process.stdout.clearLine is not a function
error during build:
TypeError: process.stdout.clearLine is not a function
    at Object.buildEnd (./node_modules/vite/dist/node/chunks/dep-59dc6e00.js:2975:36)
    at ./node_modules/rollup/dist/shared/rollup.js:23038:37
    at async Promise.all (index 2)
    at async ./node_modules/rollup/dist/shared/rollup.js:23840:13
    at async catchUnfinishedHookActions (./node_modules/rollup/dist/shared/rollup.js:23342:20)
    at async rollupInternal (./node_modules/rollup/dist/shared/rollup.js:23830:5)
    at async doBuild (./node_modules/vite/dist/node/chunks/dep-59dc6e00.js:41402:24)
    at async build (./node_modules/vite/dist/node/chunks/dep-59dc6e00.js:41244:16)
    at async CAC.<anonymous> (./node_modules/vite/dist/node/cli.js:738:9)

If you run yarn vite build instead then you are able to run preview command.

$ deno run --no-check --unstable --compat -A ./node_modules/.bin/vite preview
  > Local: http://localhost:4173/
  > Network: use `--host` to expose

Also, if you add a config file vite.config.ts:

import { defineConfig } from "vite";
import react from "@vitejs/plugin-react";

export default defineConfig({
  plugins: [react()]
});

it will crash:

$ deno run --no-check --unstable --compat -A ./node_modules/.bin/vite
failed to load config from ./vite.config.ts
error when starting dev server:
Error: Not implemented: ChildProcess.unref()
    at notImplemented (https://deno.land/std@0.141.0/node/_utils.ts:22:9)
    at ChildProcess.unref (https://deno.land/std@0.141.0/node/internal/child_process.ts:238:5)
    at ensureServiceIsRunning (./node_modules/esbuild/lib/main.js:2067:9)
    at Object.build (./node_modules/esbuild/lib/main.js:1935:26)
    at bundleConfigFile (./node_modules/vite/dist/node/chunks/dep-59dc6e00.js:61872:34)
    at loadConfigFromFile (./node_modules/vite/dist/node/chunks/dep-59dc6e00.js:61849:35)
    at resolveConfig (./node_modules/vite/dist/node/chunks/dep-59dc6e00.js:61376:34)
    at createServer (./node_modules/vite/dist/node/chunks/dep-59dc6e00.js:59783:26)
    at CAC.<anonymous> (./node_modules/vite/dist/node/cli.js:688:30)

watch me reproduce the whole list here

@kt3k
Copy link
Member

kt3k commented May 30, 2022

vite compatibility is also tracked in this issue denoland/std#2055

For ChildProcess.ref/unref APIs, we first needs to implement ref and unref methods to Deno.Child and then implement ChildProcess.ref/unref in std/node's ChildProcess

vite is also blocked by upgrade event of http.Server in std/node, which is being worked on denoland/std#2157

@bartlomieju
Copy link
Member

Thanks for looking into it @JLarky. Last time I checked (~February) I was able to run vite dev and have HMR in the browser. It looks like Vite has changed a bit since then and we got more APIs missing.

@JLarky
Copy link

JLarky commented May 30, 2022

vite compatibility is also tracked in this issue denoland/std#2055

@kt3k thank you for the link! that would have helped ^_^

Thanks for looking into it @JLarky. Last time I checked (~February) I was able to run vite dev and have HMR in the browser. It looks like Vite has changed a bit since then and we got more APIs missing.

Interesting! I will check if I can find an older version of vite that is compatible with Deno.

Also, I don't know if this is on topic, but I couldn't find this information anywhere. Is there a way to run npm install, npx kind of commands with Deno? Because it seems like you are expected to run yarn create vite or yarn add vite so you have to have node.js installed.

@kt3k
Copy link
Member

kt3k commented May 31, 2022

If you use mac or linux, you can replace node with the below shell script

#!/bin/sh
deno run --no-check --unstable --compat -A $@

And the commands like yarn ..., npm ..., etc can run under node compat mode. (But this is still very experimental. Please don't expect everything works with this

@sunw31
Copy link

sunw31 commented Jun 16, 2022

vite compatibility is also tracked in this issue denoland/deno_std#2055

@kt3k thank you for the link! that would have helped ^_^

Thanks for looking into it @JLarky. Last time I checked (~February) I was able to run vite dev and have HMR in the browser. It looks like Vite has changed a bit since then and we got more APIs missing.

Interesting! I will check if I can find an older version of vite that is compatible with Deno.

Also, I don't know if this is on topic, but I couldn't find this information anywhere. Is there a way to run npm install, npx kind of commands with Deno? Because it seems like you are expected to run yarn create vite or yarn add vite so you have to have node.js installed.

just wait to see WinterCG's job, such as universe the api first target gradually.imaging the last result:

nodejs-------sync-----------polyfill-----------little difference, but not popular anymore
deno---------sync----------adapter----------very popular at the right way...
browser------sync----------plus dom--------easy to reuse webapi experience
worker-------sync----------adapter----------easy to copy and paste code

@benjamingr
Copy link
Contributor

Hi, Node streams V4 with better readable stream support was released - it'd be neat if Deno used it instead of its own implementation of Node streams since it contains a bunch of bug fixes and new APIs.

@bartlomieju
Copy link
Member

Hi, Node streams V4 with better readable stream support was released - it'd be neat if Deno used it instead of its own implementation of Node streams since it contains a bunch of bug fixes and new APIs.

Thanks for the heads up @benjamingr, could you link to relevant bits that we could reuse?

@benjamingr
Copy link
Contributor

This is the release https://github.com/nodejs/readable-stream/releases/tag/v4.0.0 - it updates Node streams to be compatible with Node.js 18 (and not 10) which contains a ton of new APIs (like iterator helpers). It should be consumable directly in Deno since it works in browsers.

@millette
Copy link

See also nodejs/readable-stream#451

@benjamingr
Copy link
Contributor

Yeah though that predates the streams V4 release

@jerry4718
Copy link

Deno has panicked.

// index.js
import Koa from 'koa';
import koaStatic from 'koa-static';
import koaBodyParser from 'koa-bodyparser';
import koaConnect from 'koa2-connect';
import { createProxyMiddleware, fixRequestBody } from 'http-proxy-middleware';
import { parseArgv } from './parse-argv.js';
import { getIPAddress } from './get-ip-address.js';

const app = new Koa();

const param = parseArgv();
console.log(param);

const koaProxyMiddleware = koaConnect(createProxyMiddleware({
    logLevel: 'debug',
    target: param.proxy_target,
    ws: true,
    changeOrigin: true,
    pathRewrite: { '/api': '' },
    onProxyReq: fixRequestBody,
}));

app.use(async (ctx, next) => {
    if (!/^\/work\//.test(ctx.url)) {
        await next();
        return;
    }
    ctx.respond = false;
    await koaProxyMiddleware(ctx, next);
});

app.use(koaStatic(param.static_dir));

app.use(koaBodyParser({ enableTypes: ['json', 'form', 'text'] }));

app.listen(param.serve_port, '0.0.0.0', () => {
    console.log('started');
    getIPAddress()
        .forEach(d => console.log(`device: ${d.name}, address: http://${d.ip}:${param.serve_port}/`));
});
// parse-argv.js
const inputArgReg = /^--[^=]+(=.*?)?$/;

export function parseArgv () {
    const inputArgv = process.argv.slice()
        .filter(arg => inputArgReg.test(arg))
        .reduce(
            (allArg, arg) => {
                if (arg.indexOf('=') > -1) {
                    const [key, value] = arg.replace(/^--/, '').split(/=/);
                    return Object.assign(allArg, { [key]: value });
                } else {
                    const key = arg.replace(/^--/, '');
                    return Object.assign(allArg, { [key]: true });
                }
            },
            {},
        );

    const {
        serve_port = 9540,
        proxy_target = 'http://host:port/api',
        static_dir = '/some/location/dir',
    } = inputArgv;

    return { serve_port, proxy_target, static_dir };
}

// get-ip-address.js
import os from 'node:os';

export function getIPAddress () {
    const interfaces = os.networkInterfaces();
    return Object.keys(interfaces)
        .map(interfaceKey => ({
            name: interfaceKey,
            ip: interfaces[interfaceKey]
                .filter(details => details.family === 'IPv4')
                .map(details => details.address)[0],
        }))
        .filter(d => d.ip);
}

Platform: linux x86_64
Version: 1.25.0
Args: ["/home/jerry/.deno/bin/deno", "run", "--compat", "--unstable", "--inspect", "--allow-all", "./node-serve/index.js", "--serve_port=9528", "--proxy_target=http://host:port/api", "--static_dir=/some/location/dir"]

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', cli/compat/mod.rs:211:70
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:48:5
   3: core::option::Option<T>::unwrap
   4: deno::proc_state::ProcState::prepare_module_load::{{closure}}
   5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   6: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   7: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   8: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   9: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  10: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  11: deno::run_command::{{closure}}
  12: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  13: deno::main::{{closure}}
  14: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  15: deno_runtime::tokio_util::run_local
  16: deno::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Process exited with code 1

@rawkakani
Copy link

Parcel uses "native addons" (.node files), so I'm not sure we'll be able to run that at all.

Parcel, Next.js, SWC are using napi-rs to build native addon.

Since napi-rs@v2 introduced high level proc macro to build native bindings, I'm planning to add WebAssembly support for it. Maybe deno FFI could be supported in this way too. Like a feature flag: 'deno', and libraries with this flag could be built for deno ffi.

How is this coming along ?

@bartlomieju
Copy link
Member

I'm going to close this issue now as we removed --compat mode and focus our efforts on better compatibility using npm: specifiers. Relevant issues for various projects will stay open for now.

@bartlomieju bartlomieju unpinned this issue Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted) node compat
Projects
None yet
Development

No branches or pull requests