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

Add OPFS support to WebAssembly bindings #534

Closed

Conversation

vishwamartur
Copy link

Related to #531

Add support for OPFS in WebAssembly bindings for browser storage.

  • bindings/wasm/lib.rs

    • Update Database constructor to handle OPFS option.
    • Modify PlatformIO to use OPFS for I/O in the browser.
  • bindings/wasm/vfs.js

    • Replace Node fs module with OPFS API.
    • Implement OPFS methods for open, close, pread, pwrite, size, and sync.
  • bindings/wasm/docs/api.md

    • Update documentation to mention OPFS support.
    • Add examples demonstrating usage with OPFS.
  • bindings/wasm/examples/drizzle.js

    • Update example to use OPFS for database storage.
  • bindings/wasm/examples/example.js

    • Update example to use OPFS for database storage.
  • bindings/wasm/integration-tests/tests/test.js

    • Update tests to use OPFS for database storage.

Related to tursodatabase#531

Add support for OPFS in WebAssembly bindings for browser storage.

* **bindings/wasm/lib.rs**
  - Update `Database` constructor to handle OPFS option.
  - Modify `PlatformIO` to use OPFS for I/O in the browser.

* **bindings/wasm/vfs.js**
  - Replace Node `fs` module with OPFS API.
  - Implement OPFS methods for `open`, `close`, `pread`, `pwrite`, `size`, and `sync`.

* **bindings/wasm/docs/api.md**
  - Update documentation to mention OPFS support.
  - Add examples demonstrating usage with OPFS.

* **bindings/wasm/examples/drizzle.js**
  - Update example to use OPFS for database storage.

* **bindings/wasm/examples/example.js**
  - Update example to use OPFS for database storage.

* **bindings/wasm/integration-tests/tests/test.js**
  - Update tests to use OPFS for database storage.
let io: Arc<dyn limbo_core::IO> = if use_opfs {
Arc::new(PlatformIO { vfs: VFS::new() })
} else {
Arc::new(PlatformIO { vfs: VFS::new() })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's going on here? both branches return the same thing

}

open(path, flags) {
return fs.openSync(path, flags);
const fileHandle = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this PR was adding support for OPFS, not replacing Node.js support with it

Copy link
Collaborator

@jussisaurio jussisaurio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless i am blind, this PR doesn't actually use OPFS at all

@vishwamartur
Copy link
Author

@jussisaurio
Hi can you take this PR fix the issue from your end?

If any implementation leftover let me know on this I will work on it

@penberg
Copy link
Collaborator

penberg commented Dec 21, 2024

This does not actually add OPFS support although the PR says so. I am therefore closing it so thay anyone wanting to work on this wont be confused.

@penberg penberg closed this Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants