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

Implicitly get bindable default value from assignment #4

Open
Justin-Lessard opened this issue Jun 1, 2018 · 5 comments
Open

Implicitly get bindable default value from assignment #4

Justin-Lessard opened this issue Jun 1, 2018 · 5 comments

Comments

@Justin-Lessard
Copy link

I would be nice that initial variable assignment would be automatically set as the default value for typed bindable/observable.

What i mean is the following syntax:

@bindable.number public myNumber: number = 123;

Should be equivalent to this one:

@bindable.number({defaultValue: 123})
public myNumber!: number;

Now the first version without the default value create the bindable with an undefined value at first, and then assign 123 to it.

This cause problems since have a strict validation in my coercion function that throws an error if I try to assign undefined.

How hard would it be to implicitly set a default value for defaultValue using the part after the =?

Thank you.

@bigopon
Copy link
Collaborator

bigopon commented Jun 9, 2018

I missed this, so sorry for the delayed answer.

The issue you described is because the way TypeScript compiles to JS, which I can't fix.

strict validation in my coercion function

May I know how you are using it ?

@Justin-Lessard
Copy link
Author

Thanks for your answer!

By strict validation function, I means I throw an error when the value is not a valid type. See a simplified example below.

coerceFunctions.number = (value: any): number => {
    const numeralValue: number = Number(value);

    if (isNaN(numeralValue)) {
        throw new Error(`"${value}" is not a valid number`);
    }

    return numeralValue;
};

Anyway this is just a minor annoyance, otherwise your plugin works flawlessly 👍
So, feel free to close this issue. Thank you for the great work!

@bigopon
Copy link
Collaborator

bigopon commented Jun 12, 2018

@Justin-Lessard What you described is also my concern, as I'm not sure whether initial value should be coerce no matter what, even when not explicitly defined:

class A {
  @bindable.number()
  a: number;
}

Should it coerce undefined (starting value) or not ? At the moment, it is coerced into NaN

@Justin-Lessard
Copy link
Author

We use your plugin to coerce value from binding to the view-model into the correct type. I don't see a case where coercing the initial value make senses.

In all case you could just explicitly declare your variable in the correct type instead of using a coerce function.

In your example, if you really needed to have NaN and not undefined you could just declare a = NaN.

But Imho, it's cleaner to explicitly assign NaN than using an initial coercing. Moreover, in your example where a is undefined, you would need to change the type to number | undefined or skip initial validation using a!: number to pass the typechecker.

Worst case, would it be hard to add a config to toggle the initial coercion on or off?

@bigopon
Copy link
Collaborator

bigopon commented Jun 13, 2018

Turning off them by default is not hard, maybe a check from exposed config here https://github.com/aurelia-contrib/aurelia-typed-observable-plugin/blob/master/src/patches.ts#L117, and here https://github.com/aurelia-contrib/aurelia-typed-observable-plugin/blob/master/src/patches.ts#L158 to disable them.

Disabling it by default sounds like a breaking change to me, probably a config is needed to turn off instead.

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

No branches or pull requests

2 participants