SkipLabs
[skstore] Allow mappers to close only over handles
This diff removes the class type argument from map functions in the SKStore API, and changes the parameters for the Mapper class constructors from effectively
...params: any[]
to
eparams: EHandle<any, any>[],
lparams: LHandle<any, any>[]
The benefit of these changes is that calls to the map functions no
longer need to specify the type of the class. This helps since type
inference is not strong enough to elaborate the type parameters
automatically. Also, specifying the types of each parameter as only
either EHandle or LHandle gives API users help to only close over
state that the SKStore incremental computation engine is aware of.
A drawback is that splitting the params into a pair of arrays rather
than one varargs list is less convenient at call sites. Additionally,
the constructor functions of mapper classes have a uniform
signature that is only very weakly typed: the number and order of the
actual class constructor parameters must be ensured manually.
This is draft PR for discussion, I don't know if we should go this way, but maybe worth looking at and thinking about.
FWIW, so far my attempts to rewrite this using the isomorphism between EHandle<any, any>[], LHandle<any, any>[] and (EHandle<any, any> | LHandle<any, any>)[] have failed. I have been able to get examples to type-check, but haven't been able to get the type-checker to reject bad code. More investigation needed.
Just for clarity, this is just one approach, and there is at least one more that I will investigate. So definitely don't spend any great amount of time reviewing this.
I think we can close this. @skiplabsdaniel has a more flexible approach that seems to make TS type-inference happy.
I think we can close this. @skiplabsdaniel has a more flexible approach that seems to make TS type-inference happy.
Yes, agreed. I think there is still the question of whether we want to try to give users some help from the type-checker to ensure that all the parameters to class constructors are handles, or if the best we can do is to only check this at runtime. Thoughts?