Page MenuHomePhabricator

[Comm-opaque2] Add wasm build to client
ClosedPublic

Authored by jon on Mar 16 2023, 4:30 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 13, 6:56 AM
Unknown Object (File)
Fri, Mar 8, 1:34 AM
Unknown Object (File)
Fri, Mar 8, 1:34 AM
Unknown Object (File)
Thu, Mar 7, 9:00 PM
Unknown Object (File)
Thu, Mar 7, 9:00 PM
Unknown Object (File)
Thu, Mar 7, 8:59 PM
Unknown Object (File)
Thu, Mar 7, 8:59 PM
Unknown Object (File)
Tue, Mar 5, 11:40 PM
Subscribers

Details

Reviewers
varun
ashoat
tomek
bartek
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rCOMMfa13b21baf73: [Comm-opaque2] Add wasm build to client
Summary

Build wasm and js bindings for the client sided opaque
code.

https://linear.app/comm/issue/ENG-3265

Depends on D7022

Test Plan
nix develop
cd shared/comm-opaque2
wasm-pack build

# pkg directory contains js, ts, and wasm artifacts
ls pkg/

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Add wasm-pack to this diff

Owners added a reviewer: Restricted Owners Package.Mar 16 2023, 5:00 PM

Add back rlib for in-repo usage

Add @ashoat because addtion of wasm-pack utility, and new usage of wasm_bindgen crate

This currently produces the outpus:

$ du -hd0 --apparent-size pkg/*
715	pkg/comm_opaque.d.ts
160	pkg/comm_opaque.js
14K	pkg/comm_opaque_bg.js
142K	pkg/comm_opaque_bg.wasm
1005	pkg/comm_opaque_bg.wasm.d.ts
249	pkg/package.json
ashoat added 1 blocking reviewer(s): varun.

Dependencies look good! No context on Rust unfortunately

varun requested changes to this revision.Mar 20 2023, 7:33 AM
varun added inline comments.
shared/comm-opaque2/Cargo.toml
11 ↗(On Diff #23808)

where does this get used?

22 ↗(On Diff #23808)

optimize

shared/comm-opaque2/src/client/login.rs
32 ↗(On Diff #23808)

why do we need the wasm_bindgen attribute on every function? other examples i looked at only had it above the impl block

shared/comm-opaque2/src/client/register.rs
29 ↗(On Diff #23808)

again don't think we need all these decorators

shared/comm-opaque2/src/error.rs
5 ↗(On Diff #23808)

I would say due to Rust's orphan rules

shared/comm-opaque2/src/lib.rs
54–55 ↗(On Diff #23803)

nit: maybe rename login_client to client_login just for consistency

This revision now requires changes to proceed.Mar 20 2023, 7:33 AM
jon marked 6 inline comments as done.

Address Feedback

shared/comm-opaque2/Cargo.toml
11 ↗(On Diff #23808)

rand brings in getrandom. It essentially fails to compile unless you specify this dependency

22 ↗(On Diff #23808)

apparently I've been around too many british :/

shared/comm-opaque2/src/client/login.rs
32 ↗(On Diff #23808)

you're right, we don't. Was just following wasm-opaque.

shared/comm-opaque2/src/lib.rs
54–55 ↗(On Diff #23803)

fair

varun added inline comments.
shared/comm-opaque2/src/error.rs
5 ↗(On Diff #23874)

can not -> cannot

This revision is now accepted and ready to land.Mar 20 2023, 1:15 PM
jon marked an inline comment as done.

typo

This revision was automatically updated to reflect the committed changes.