-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Add a type option to bindable for type conversion #96
Comments
Absolutely. One of the ideas I have is to let you specify a ValueConverter as part of your bindable definition. This would then enable arbitrary type conversion to be built in to the property. To simplify common use cases, you could specify a type, and then we would select a built-in converter based on that type. In the case of TypeScript, we can even leverage the type metadata to do this automatically. Thoughts? |
yes using some common defaults and then able to pass custom value converter seems much more flexible :) |
@Aaike I'm thinking of something like this: For TS, we could write:
Assuming they turned on TS metadata, we could interpret that as this:
For ES6 we could enable something like this:
Thoughts? |
Those defaults look great yes, and this also still allows custom converers for ES6 in the following form i assume ?
|
Yes. That's my thought. |
If I was taking the lazy way and tried to bind a complex JSON object into my viewmodel, would I have to create a custom valueconverter or define a nested I think I'd prefer something like |
I think the datatype to the binding signature would be a great feature. Just discovered our numeric inputs are converting to strings so this would have been nice to coerce the binding without having to use a value converter |
@brettveenstra / @jadrake75 would you agree that |
Apologies if I misunderstand, but I don't think a ValueConverter (used in a view) is a substitute for being able to specify a data type in a @bindable (say inside a CustomElement) because as a CustomElement author I don't want consumers of the CustomElement to need to remember to use the ValueConverter to pass the correct type. |
@csaloio right- I was specifically talking about this comment #96 (comment) by @brettveenstra. |
Ah gotcha. Thought I might have misunderstood :) |
I agree.... the ability to bind the value to a type independently of how it is used as part of the view-model definition is quite powerful. Note having some simple valueConverters in the framework would help anyways but I think it is a different strategy then encoding it with the binding. I myself have used ones for empty values, date formats, number conversions, currencies, object to array, upper casing, amongst others. |
i've also recognised when going beyond the skeleton (or starter) - projects there is a high chance to need a value converter. in my recent test app i've ended up using a value converter for every input field. so i see some potential there for optimization as well (doing it in the bindable) |
@EisenbergEffect I don't think we should by default infer that the user wants a given @bindable @enforce('string') property; and with the case of metadata from TypeScript: @bindable @autoenforce property: string; Where I also think it would be great to be able to also pass a function to act as a simple custom enforcer using the same syntax, e.g. integer enforcer: @bindable @enforce(value => parseInt(value)) property; |
I agree it may not be good to automatically infer the type using TypeScript metadata. How about an API like this: declare type CoerceInstruction = 'String' | 'Number' | 'Date' | { (value: any): any };
declare BindableConfig {
name?: string;
defaultBindingMode?: BindingMode;
changeHandler? string;
// new:
coerce?: CoerceInstruction;
} export class Foo {
@bindable({ coerce: 'number' }) bar;
@bindable({ coerce: x => +x }) baz;
} I think this would closely mirror what built-ins like input value do in terms of type coercion. |
I like @jdanyow's proposal. CoerseInstruction strings should be lowercase though: type CoerceInstruction = 'string' | 'number' | 'date' | { (value: any): any }; |
Yes, I'd prefer to not add another decorator if possible :) But, what about the fluent syntax I propose above (which could just use the @bindable.string firstName;
@bindable.boolean happy; |
@EisenbergEffect There's also aurelia/binding#347 (comment) to consider - it's essentially the same as this issue, but focused more on @niieani, @jdanyow I'm not sure I understand why there would be any issue with inferring the type based on TypeScript metadata - the fact that the type is not inferred is what currently leads to "crazy errors that will be really hard to trace". In what scenario would you ever want the actual value to have a different type than the one declared? I really think this needs to be inferred. Of course, there's limits to what can be inferred - it probably shouldn't try to support e.g union types, but making this "just work" for the 90% case of simple types like Personally, I think the suggestion in #96 (comment) by @EisenbergEffect is absolutely the way to go. My only concern is that it should maybe be debated whether |
I also like @jdanyow's proposal. |
Does something like this exist now? |
I created decorators for that, but somehow the @bindable attribute completely breaks my custom decorators. See https://blog.rsuter.com/angular-2-typescript-property-decorator-that-converts-input-values-to-the-correct-type/ I've create a new issue for that: #546 |
@bigopon thank you for working on this, however I am not convinced this is required to be built in the framework. Aurelia does not provide any value converters so why would the coerce functions be provided? Can all this functionality be achived by using a decorator? This was previously suggested here. Here is a naive, untested implementation: function convert(converter) {
return function (target, key) {
Object.defineProperty(target, key, {
get: function () {
return this["__" + key];
},
set: function (newValue) {
this["__" + key] = converter(newValue);
},
enumerable: true,
configurable: true
});
};
}
class Person {
@bindable
@convert(String)
name: string;
constructor(name: string) {
this.name = name;
}
} |
Aurelia For |
When this will be released?????? None of the below currently works (I am using @bindable.number number1;
@observable.number number2; @bindable({ coerce: 'number' }) number1;
@observable({ coerce: 'number' }) number2; This issue #96 is closed without a clear solution. Because anyone who used aurelia for some time would had definetly come across this issue. Also ValueConvertors and BindingBehavior don't cut it. The solution that seems the cleanest is the one proposed by @EisenbergEffect here. 👍 I see there are attempts to address this in the following pull requests (and good efforts by @bigopon ): So, if this will take more time before it gets released, what is the current approach to support this: <input type="number" value.bind="number1" /> So that <input type="number" value.bind="number1 & validate" /> 🙏 ? |
You can add a value converter to <input value.bind="number1 | number" /> with number is a value converter: export class NumberValueConverter {
fromView(val) {
let number = Number(val);
if (!isNaN && isFinite(number)) return number;
return 0;
}
} |
good candidate for contrib maybe can be closed |
There's ongoing work for this. @bigopon What is the status? Are there still some issues? |
All work to support this was done. It can be merged anytime. Still only main block is the concern over the confusion it may give when values in view and view model don't match, especially when people use non primitive typed bindables. Aurelia binding is pretty clean at the moment, so this concerns @jdanyow |
Love to get some feedback from @jdanyow |
Just ran into this. Especially when using TypeScript this is quite a bit counter-intuitive; you get string instances coming through your type system as numbers. |
@jnm2 all input values are strings (-: |
I expected Aurelia to know about the statically-typed model it was binding and to |
Hmmm... It is not that statically typed at runtime (-: Don't be upset about your rookie move. |
I know that reflection is a thing with TypeScript, so I know in theory Aurelia could act like the model is statically typed at runtime if the tooling put it all together. I still think it should either do this reflection at runtime like I expected, or do this reflection at compile time and emit a warning because the type needed to be assignable from |
A solution now exists in a form of a plugin |
can be closed with a plugin |
I was wondering if it would be a good idea to add an option to the
@bindable
decorator to make it convert a value to a different type than a string.This can be useful when creating a custom element that accepts numeric or boolean options :
Right now the values would have to be manually converted every time the variable is used.
perhaps an option could be added like this:
The text was updated successfully, but these errors were encountered: