opendal icon indicating copy to clipboard operation
opendal copied to clipboard

Can we make opendal-nodejs running on browser or electron?

Open Xuanwo opened this issue 1 year ago • 14 comments

As requested by https://github.com/apache/incubator-opendal/issues/3803#issuecomment-1873170368, can we make merge wasm support with opendal-nodejs so that we can run on browser or electron?

For example, aws-crt is written in C, but also provide browser support: https://www.npmjs.com/package/aws-crt

Maybe via napi's native wasm support?

https://github.com/napi-rs/napi-rs/blob/03eb476cef61fb15f1ec071c415cbf6fbfa8e830/crates/napi/Cargo.toml#L78-L87

Xuanwo avatar Jan 01 '24 08:01 Xuanwo

Invite @suyanhanx, @fyears, @Brooooooklyn to join in the discussion.

Xuanwo avatar Jan 01 '24 08:01 Xuanwo

Current progress:

:( NAPI_TARGET=wasm32-unknown-unknown pnpm run build

> [email protected] build /home/xuanwo/Code/apache/incubator-opendal/bindings/nodejs
> napi build --platform --features "${NAPI_FEATURES:-}" --target "${NAPI_TARGET:-}" --release --js generated.js --dts generated.d.ts && node ./scripts/header.js
   ...
   Compiling opendal-nodejs v0.44.1 (/home/xuanwo/Code/apache/incubator-opendal/bindings/nodejs)
error: future cannot be sent between threads safely
   --> bindings/nodejs/src/lib.rs:772:1
    |
772 | #[napi]
    | ^^^^^^^ future created by async block is not `Send`
    |
    = help: the trait `std::marker::Send` is not implemented for `(dyn futures::Future<Output = (std::string::String, std::result::Result<opendal::Metadata, opendal::Error>)> + 'static)`
note: captured value is not `Send` because `&mut` references cannot be sent unless their referent is `Send`
   --> bindings/nodejs/src/lib.rs:772:1
    |
772 | #[napi]
    | ^^^^^^^ has type `&mut Lister` which is not `Send`, because `Lister` is not `Send`
note: required by a bound in `execute_tokio_future`
   --> /home/xuanwo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/napi-2.14.1/src/tokio_runtime.rs:121:18
    |
119 | pub fn execute_tokio_future<
    |        -------------------- required by a bound in this function
120 |   Data: 'static + Send,
121 |   Fut: 'static + Send + Future<Output = Result<Data>>,
    |                  ^^^^ required by this bound in `execute_tokio_future`
    = note: this error originates in the attribute macro `napi` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `opendal-nodejs` (lib) due to previous error
(node:30467) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Internal Error: Command failed: cargo build --release --target wasm32-unknown-unknown
    at genericNodeError (node:internal/errors:956:15)
    at wrappedFn (node:internal/errors:510:14)
    at checkExecSyncError (node:child_process:890:11)
    at Object.execSync (node:child_process:962:15)
    at BuildCommand.<anonymous> (/home/xuanwo/Code/apache/incubator-opendal/bindings/nodejs/node_modules/.pnpm/@[email protected]/node_modules/@napi-rs/cli/scripts/index.js:11555:30)
    at Generator.next (<anonymous>)
    at /home/xuanwo/Code/apache/incubator-opendal/bindings/nodejs/node_modules/.pnpm/@[email protected]/node_modules/@napi-rs/cli/scripts/index.js:3552:69
    at new Promise (<anonymous>)
    at __awaiter$1 (/home/xuanwo/Code/apache/incubator-opendal/bindings/nodejs/node_modules/.pnpm/@[email protected]/node_modules/@napi-rs/cli/scripts/index.js:3548:10)
    at BuildCommand.execute (/home/xuanwo/Code/apache/incubator-opendal/bindings/nodejs/node_modules/.pnpm/@[email protected]/node_modules/@napi-rs/cli/scripts/index.js:11325:16)
 ELIFECYCLE  Command failed with exit code 1.

Xuanwo avatar Jan 01 '24 08:01 Xuanwo

@Xuanwo you can assign this issue to me :) I'm working on wasm32-wasi support in NAPI-RS

Brooooooklyn avatar Jan 02 '24 13:01 Brooooooklyn

@Xuanwo you can assign this issue to me :) I'm working on wasm32-wasi support in NAPI-RS

Thank you for participating in this! If you need any further assistance or help, feel free to let us know.

suyanhanx avatar Jan 02 '24 14:01 suyanhanx

@Xuanwo you can assign this issue to me :) I'm working on wasm32-wasi support in NAPI-RS

FYI, the opendal is not support wasm32-wasi yet, I'm trying to do a min support

Zheaoli avatar Jan 02 '24 14:01 Zheaoli

Supporting wasm32-wasi is cool, but it seems unrelated to the goal of running in a browser. For example, supporting the use case like remotely-save

Xuanwo avatar Jan 02 '24 16:01 Xuanwo

Supporting wasm32-wasi is cool, but it seems unrelated to the goal of running in a browser. For example, supporting the use case like remotely-save

If we just want to make the opendal useable in electron. I think it already be done

image

image

Zheaoli avatar Jan 02 '24 16:01 Zheaoli

If we just want to make the opendal useable in electron. I think it already be done

It seems more complex while on mobile as described in https://github.com/remotely-save/remotely-save/blob/master/docs/browser_env.md

Xuanwo avatar Jan 02 '24 17:01 Xuanwo

It seems more complex while on mobile as described in

The mobile is totally different with the desktop. All of the hybrid mobile solutions is depend on the webview which is provided by the platform. Which is means that we need solve issue case by case(It's already not a issue related with the browser or electron)

If we just want to run opendal on the capacitorjs which is used by the plugin you mentioned, I might have some trick to run the workflow as normal.

Zheaoli avatar Jan 02 '24 17:01 Zheaoli

electron is just like nodejs and should work by calling nodejs library. browser is different.

If we just want to run opendal on the capacitorjs

no need to consider that. let’s focus on the general browser environment firstly

fyears avatar Jan 02 '24 21:01 fyears

Hi, @fyears, I'm now using remotely-save! Fantastic job! Let's see how we can improve it by using opendal.

Xuanwo avatar Feb 18 '24 12:02 Xuanwo

Following the discussion at https://github.com/napi-rs/napi-rs/issues/1794, it seems we just need to wait for napi 3.0 and add wasm32 target during release.

Xuanwo avatar Feb 18 '24 12:02 Xuanwo

cc @suyanhanx, after this implemented, we will need to rename bindings/nodejs to bindings/js?

Xuanwo avatar Feb 20 '24 07:02 Xuanwo

cc @suyanhanx, after this implemented, we will need to rename bindings/nodejs to bindings/js?

I think it's okay to rename.

suyanhanx avatar Feb 20 '24 07:02 suyanhanx