[web] Deprecate non-worker opaque wasm
ClosedPublic
Actions

Authored by michal on Mar 19 2024, 7:34 AM.

Details

Reviewers

kamil
tomek

Commits

rCOMM09126feb26d5: [web] Deprecate non-worker opaque wasm

Summary

ENG-7242 : Deprecate opaque WASM for non-worker web context

D11274, as a part of moving the identity client to the shared worker, introduces a new way to host the opaque WASM file for the webworker that is more inline with the rest of the WASM files. This diff removes the previous WASM file hosted for the browser tab JS context.

Depends on D11351

Test Plan

Made sure that in dev and prod there were no errors in the console after loading the webapp.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

michal created this revision.Mar 19 2024, 7:34 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMar 19 2024, 7:34 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 19 2024, 7:38 AM

Harbormaster failed remote builds in B27581: Diff 38170!

Harbormaster completed remote builds in B27581: Diff 38170.Mar 19 2024, 7:50 AM

michal requested review of this revision.Mar 19 2024, 7:50 AM

kamil accepted this revision.Mar 19 2024, 8:57 AM

kamil added inline comments.

web/crypto/opaque-utils.js
20 ↗	(On Diff #38170)	I guess you can move this file to `/worker` directory

This revision is now accepted and ready to land.Mar 19 2024, 8:57 AM

Move file

Harbormaster completed remote builds in B27598: Diff 38189.Mar 19 2024, 9:54 AM

ashoat added inline comments.Mar 19 2024, 5:59 PM

web/app.react.js
89 ↗	(On Diff #38189)	Why was this line removed?

michal added inline comments.Mar 20 2024, 4:27 AM

web/app.react.js
89 ↗	(On Diff #38189)	We no longer use opaque in the browser-tab-JS-context and only in worker (the only places where it is used is `web/grpc/identity-service-client-wrapper.js` which now is only created on the worker). We longer need to initialize it on the tab context.

ashoat added inline comments.Mar 20 2024, 11:23 AM

web/app.react.js
89 ↗	(On Diff #38189)	Thanks for explaining! I think the reason this line was here in the first place is to try to initialize the module as soon as possible, instead of lazily. Would it be good to have a similar line in the web worker? Or is it not necessary?