Skip to content

Conversation

@bigopon
Copy link
Member

@bigopon bigopon commented Aug 1, 2017

This PR adds coerce support for decorator observable.

Usage:

With normal syntax

class MyViewModel {
  @observable({ coerce: 'number' }) numProp
  @observable({ coerce: 'boolean' }) boolProp
  @observable({ coerce: val => convertValue(val) }) customCoerceProp
}

Using metadata

import {metadata} from 'aurelia-metadata';

class MyViewModel {
  @observable
  @Reflect.metadata(metadata.propertyType, Number)
  num
}

var instance = new MyViewModel();
instance.num = '4';
instance.num; // <====== 4

TypeScript users can achieve above result (metadata) by simpler code:

class MyViewModel {
  @observable num: number
}

var instance = new MyViewModel();
instance.num = '4';
instance.num; // <===== 4 , with compile error though

Remember to instruct the compiler to emit decorator metadata TypeScript decorator doc

TypeScript includes experimental support for emitting certain types of metadata for declarations that have decorators. To enable this experimental support, you must set the emitDecoratorMetadata compiler option either on the command line or in your tsconfig.json:

All coerce type will be resolved to a string, which then is used to get the converter function in coerces export of this module. So, to extend or modify basic implementations:

import {coerces} from 'aurelia-binding';

// Modify built in
coerces.string = function(a) {
  return a === null || a === undefined ? '' : a.toString();
}

// Extend
coerces.point = function(a) {
  return a.split(' ').map(parseFloat).slice(0, 2);
}

// Usage of 'point' coerces defined above:
class MyLine {
  @observable({ coerce: 'point' }) point1
  @observable({ coerce: 'point' }) point2
}

For TS users or JS users who want to use metadata, to extend coerce mapping:

import {
  mapCoerceForClass // <=== this API will be tweaked. I'm still learning how to name properly
} from 'aurelia-binding';

// use static class method
class Point {
  static coerce(value) {
    return new Point(value);
  }
}
mapCoerceForClass(Point, 'point');

// or just pass a 3rd parameter:
mapCoerceForClass(Point, 'point', val => new Point(val));

// decorator usage:
// TypeScript
class MyLine {
  @observable point1: Point
  @observable point2: Point
}
// JavaScript
class MyLine {
  @observable
  @Reflect.metatata('design:type', Point)
  point1

  // or like thi
  @observable({ coerce: 'point' })
  point2
}

With fluent syntax

class MyLine {
  @observable.number x1
  @observable.number x2

  @observable.number y1
  @observable.number y2
}

var line = new MyLine();

line.x1 = '15'
line.x1; // <======= 15

To built your own fluent syntax observable:

import {
  coerces,
  registerTypeObservable // <=== API will be tweaked
} from 'aurelia-binding'

coerces.point = function(value) {
  return value.split(' ').map(parseFloat);
}
registerTypeObservable('point');

// usage:
class MyLine {
  @observable.point point1;
  @observable.point point2;
}

@EisenbergEffect The core is in coerce.js (original PR)

@atsu85
Copy link

atsu85 commented Aug 2, 2017

oh, this PR looks a lot like aurelia/templating#558 - @bigopon, could You take a look at the comments i wrote there (don't want to duplicate them here)

@bigopon
Copy link
Member Author

bigopon commented Aug 2, 2017

The coerces module will be moved to aurelia-binding, while the rest remains in the templating as requested by @EisenbergEffect .. Thats why it seems duplicated to you. I'll have a look at the comments now.

@atsu85
Copy link

atsu85 commented Aug 2, 2017

Yes, actually i understood it (saw the related comments from other PR) - just didn't express myself very clearly with previous comment ;)

@bigopon
Copy link
Member Author

bigopon commented Aug 2, 2017

@atsu85 thanks for the review, I've put some response 😄

@bigopon
Copy link
Member Author

bigopon commented Aug 3, 2017

Demo of the decorators (incomplete) Gist

The basic work on coerce is done, (which is the easier part), but when integrated with templating, it passed the coerced value to binding, thus triggering update back to all input, which will reset the cursor, or will turn boolean input fields to either true or false string.

I've been looking into the ast to see if there is any easy solution that I can do, but no luck so far. One thing I thought of is to iterate through the subscribers and add ValueConverter to any Binding that has ValueAttributeObserver, but it looks ... bad, plus, there is noway to loop at the moment. And it may even requires to modified the hasSubscriber to check if subscribers of a particular class exists.

@jdanyow @EisenbergEffect @atsu85 Would love to have some help on this.

@atsu85
Copy link

atsu85 commented Aug 6, 2017

@bigopon, sadly i'm not familiar enough with the codebase, to help You to get moving again

@jdanyow
Copy link
Contributor

jdanyow commented Aug 12, 2017

Is there a issue this PR fixes? I'm trying to refresh my memory on the backstory for this change.

@bigopon
Copy link
Member Author

bigopon commented Aug 12, 2017

@jdanyow

This is the original issue aurelia/templating#96

Where you suggested the @bindable({ coerce: 'number' }) syntax.

This is my first attempt aurelia/templating#558

Where I tried to introduce naive coerce function into setValue of BehaviorPropertyObserver

This is my lastest try with type coercion https://gist.run/?id=ad523ef690e40e9c5cde123cd0c532fa

Where it doesn't update DOM node again after coercing value from element input event, such as input. I also described the attempt in detail in gitter, but wouldn't mind if I need to repeat it here.

TL;DR: the main requirement I couldn't overcome (completely) was the ability to not trigger update DOM again after getting the value from event, converting it and assign via setValue.

@EisenbergEffect
Copy link
Contributor

@jdanyow This is related to the other PR I just pinged you on.

@bigopon
Copy link
Member Author

bigopon commented Sep 7, 2017

I messed up my master branch so this commit is gone. Will reopen if necessary and after we sort out how to deal with @bindable in templating.

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.

4 participants