Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update model.start() type for two-way reactive functions #314

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

sophiasam96
Copy link
Contributor

Update model.start() type ModelFn to accept an object with get/set methods as an alternative to the function, rather than an alternate return type of that function.

set type also should allow return values as per the docs:
https://derbyjs.github.io/derby/models/reactive-functions#two-way-reactive-functions

sophiasam96 and others added 3 commits September 11, 2024 11:14
- Allow `set()` to return a partial array covering first N inputs to update, and remove extra brackets in `Ins[]` return type since `Ins` is already an array
- Individually validate types of each item of of `{ 0: in0, 2: in2, ... }` object return value, instead of using an index signature that allows values to be any of the inputs' types
Copy link
Contributor

@ericyhwang ericyhwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the type fixes! The Racer tests are still in JS, so we don't have any type-sensitive tests set up yet, or that would've caught the typings bug.

I spotted one minor issue with the new typings, where Ins[] shouldn't have the brackets since Ins is already an array type.

While I was at it, I played around with typings a bit to see if I could come up with something stricter for the set() object return type that validates each key-value in the object individually instead of using a broad index signature.

I found a good approach, and went ahead and pushed the change here.

This TS Playground link demonstrates the types.

Relevant demo reproduced here:

modelFn({
    get(in0: number, in1: string, in2: object) {},
    // get(in0: number, in1: string) {},  // Invalid, inputs must be same arity between get and set
    // get(in0: number, in1: boolean, in2: object) {},  // Invalid, inputs must be same types between get and set
    set(out: unknown, in0: number, in1: string, in2: object) {
        switch (out) {
            case 'A': return [42, 'abc', {}];  // Set all 3 inputs
            case 'B': return [42, 'abc'];  // Set inputs 0 and 1
            case 'C': return { 1: '' };  // Set only input at index 1
            case 'D': return { 1: '', 2: {} };  // Set only inputs at 1 and 2
            // case 'F': return ['oops'];  // Invalid, type of first input must be number
            // case 'F': return { 1: -111 };  // Invalid, type of index 1 must be string
        }
        return null;
    },
});

@ericyhwang ericyhwang merged commit 9a373dd into derbyjs:master Sep 13, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants