Page MenuHomePhabricator

[web] add types for WASM Module
ClosedPublic

Authored by kamil on Jul 19 2023, 2:51 AM.
Tags
None
Referenced Files
F2158581: D8545.diff
Mon, Jul 1, 4:11 AM
F2157624: D8545.id28935.diff
Mon, Jul 1, 2:11 AM
F2157622: D8545.id28929.diff
Mon, Jul 1, 2:11 AM
F2157620: D8545.id28816.diff
Mon, Jul 1, 2:11 AM
F2157582: D8545.diff
Mon, Jul 1, 2:08 AM
Unknown Object (File)
Mon, Jun 24, 6:28 AM
Unknown Object (File)
Sat, Jun 15, 6:09 AM
Unknown Object (File)
Thu, Jun 6, 1:11 PM
Subscribers

Details

Summary

Based on

Depends on D8544

Test Plan

flow

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
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 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

This revision was automatically updated to reflect the committed changes.

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