[web] add types for WASM Module
ClosedPublic
Actions

Authored by kamil on Jul 19 2023, 2:51 AM.

Details

Reviewers

michal
tomek

Commits

rCOMM97d89c03ccdb: [web] add types for WASM Module

Summary

Based on

Module API
TS definition
inspection of printed module
class definition

Depends on D8544

Test Plan

flow

Diff Detail

Repository

rCOMM Comm

Branch

land-1

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Jul 19 2023, 2:51 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptJul 19 2023, 2:51 AM

kamil added inline comments.Jul 19 2023, 2:54 AM

web/database/types/module.js
30 ↗	(On Diff #28816)	docs I wasn't able to find information about `what` type
70 ↗	(On Diff #28816)	I decided to put this here, not in `flow-typed`, because here we will have the `SQLiteQueryExecutor` type, which uses types from `lib`, so this is not a regular libdef

kamil added a child revision: D8547: [web] add function to retrieve WASM module.Jul 19 2023, 2:56 AM

Harbormaster completed remote builds in B21021: Diff 28816.Jul 19 2023, 3:08 AM

kamil published this revision for review.Jul 19 2023, 8:56 AM

comm-query-creator.js filename doesn't match the class inside

web/database/types/module.js
10–19 ↗	(On Diff #28816)	Can these be read only?
27 ↗	(On Diff #28816)	In the docs it's only an array of functions. Should we stick to the typescript defs (and remove the singular function option)?
30 ↗	(On Diff #28816)	Can we type as `mixed`?
32 ↗	(On Diff #28816)	Shouldn't this be a singular function?

This revision is now accepted and ready to land.Jul 20 2023, 7:37 AM

address review

web/database/types/module.js
27 ↗	(On Diff #28816)	yeah, I'll use only array of functions (I think docs are more accurate)
30 ↗	(On Diff #28816)	sure
32 ↗	(On Diff #28816)	yes, you're right

Harbormaster completed remote builds in B21112: Diff 28929.Jul 21 2023, 7:35 AM

Closed by commit rCOMM97d89c03ccdb: [web] add types for WASM Module (authored by kamil). · Explain WhyJul 21 2023, 7:39 AM

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMM97d89c03ccdb: [web] add types for WASM Module.

Same feedback as here... best to avoid class and interface if possible. Sometimes they are needed for inheritance reasons, but it doesn't look like it here