Skip to content

Comments

[Docs] Fix property type - function case#370

Closed
vicb wants to merge 1 commit intolit:masterfrom
vicb:patch-1
Closed

[Docs] Fix property type - function case#370
vicb wants to merge 1 commit intolit:masterfrom
vicb:patch-1

Conversation

@vicb
Copy link
Contributor

@vicb vicb commented Dec 14, 2018

@vicb vicb requested a review from sorvell as a code owner December 14, 2018 02:09
@ghost
Copy link

ghost commented Dec 14, 2018

@kenchris @justinfagnani Could you comment on this issue please?

My understanding from what @vicb points out is that this happens:

const MyObject = { 
  toAttribute: /* ... custom toAttribute */ 
  fromAttribute: /* ... custom fromAttribute */ 
};
const myFunction = /* ... myFunction */ ;
//...
// property declarations:
{ x: { type: Number } }  // serializer is String constructor, deserializer is Number constructor
{ y: { type: MyObject } } // serializer is custom toAttribute, deserializer is custom fromAttribute
{ z: { type: myFunction } } // serializer is String constructor, deserializer is myFunction

We also have:

// don't do this
{ w: { type: Object } } 
// Serializer is String constructor
// Deserializer is Object constructor, produces something weird e.g. { 0: "{", 1: "a", 2: ":", 3: "'", ... } 

Just wanted to check with you both because of this commit: b4c5f51

@ghost
Copy link

ghost commented Dec 14, 2018

@sorvell fyi

@ghost
Copy link

ghost commented Dec 15, 2018

Just saw this #369 which probably changes things

@vicb
Copy link
Contributor Author

vicb commented Dec 15, 2018

@katejeffreys yep it should change. I added a note there because it should have been updated.

Feel free to close this PR (maybe after #369 is updated ?)

@kevinpschaaf
Copy link
Member

@vicb @katejeffreys I just added some more info regarding the change in #369 to #375 (comment)

@vicb
Copy link
Contributor Author

vicb commented Dec 15, 2018

lg

@vicb vicb closed this Dec 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants