can/map/define default behaviors with "*" #1373

Closed
imjoshdean opened this Issue Dec 16, 2014 · 14 comments

Comments

Projects
None yet
6 participants
@imjoshdean
Contributor

imjoshdean commented Dec 16, 2014

It would be really nice if we could create default behaviors for map attributes. For example, I have an AppState object tied to my can.route; however, not every attribute I want to throw on the AppState should be serialized and put into the route. Right now I have to define every attribute I want to put in my AppState with:

attributeName: {
    serialize: false
}

What would be nicer to have would be to have a catch all definition for attribute behaviors, say, "*", that would provide a set of behaviors for all attributes unless otherwise defined. So for example:

serializeOne: {
    serialize: true
},
serializeTwo: {
    serialize: true
},
"*": {
    serialize: false,
    type: "string"
}

serializeOne and serializeTwo would be typed as strings and be serializable; all other attributes on the Map wouldn't be serialized but will be also typed as strings.

@imjoshdean imjoshdean self-assigned this Dec 16, 2014

@mtovino

This comment has been minimized.

Show comment
Hide comment

mtovino commented Dec 16, 2014

+1

@zkat

This comment has been minimized.

Show comment
Hide comment
@zkat

zkat Dec 16, 2014

Contributor

I like it. +1

Contributor

zkat commented Dec 16, 2014

I like it. +1

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Dec 21, 2014

Contributor

This wil be probably usefull.

Contributor

whitecolor commented Dec 21, 2014

This wil be probably usefull.

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Dec 22, 2014

Contributor

Word. I think I'll quick throw something together, add some tests, and make some documentation modifications. 👍

Contributor

imjoshdean commented Dec 22, 2014

Word. I think I'll quick throw something together, add some tests, and make some documentation modifications. 👍

imjoshdean added a commit that referenced this issue Dec 28, 2014

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Dec 28, 2014

Contributor

Currently sitting on a solution, but the tests are now failing in YUI - though I can't be exactly sure why. Will have to take a look later.

Contributor

imjoshdean commented Dec 28, 2014

Currently sitting on a solution, but the tests are now failing in YUI - though I can't be exactly sure why. Will have to take a look later.

@daffl daffl added this to the 2.2.0 milestone Feb 4, 2015

@daffl daffl changed the title from Proposal: can/map/define default behaviors with "*" to can/map/define default behaviors with "*" Feb 9, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 11, 2015

Contributor

@daffl @imjoshdean what would it take to get this in for can.List as well?

Contributor

justinbmeyer commented Feb 11, 2015

@daffl @imjoshdean what would it take to get this in for can.List as well?

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Feb 14, 2015

Contributor

Is * going to afffect ALL the attributes or only that are in define object?

Contributor

whitecolor commented Feb 14, 2015

Is * going to afffect ALL the attributes or only that are in define object?

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Feb 16, 2015

Contributor

@whitecolor It will affect all attributes - but it will have the same minimal footprint that define has already.

Contributor

imjoshdean commented Feb 16, 2015

@whitecolor It will affect all attributes - but it will have the same minimal footprint that define has already.

@daffl daffl removed the fixed in branch label Feb 20, 2015

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Apr 10, 2015

Contributor

Is this supposed to work? In master?

Contributor

whitecolor commented Apr 10, 2015

Is this supposed to work? In master?

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Apr 10, 2015

Contributor

Yes. But @imjoshdean still needs to add documentation (#1449)

Contributor

daffl commented Apr 10, 2015

Yes. But @imjoshdean still needs to add documentation (#1449)

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Apr 10, 2015

Contributor

http://jsfiddle.net/xKc3H/367/

var Model = can.Model.extend({

    define: {
        name: {
            value: 'Alex'            
        },
        '*': {
            serialize: false
        }
    }
})


var model = new Model({age: 10, name: 'John'})

console.log(model.serialize())

Why result is Object {name: "John"} ? I thought no props should be serialized, no? It is not very useful if * defines only not defnied attributes, I think it should aslo define each attribute properties.

Contributor

whitecolor commented Apr 10, 2015

http://jsfiddle.net/xKc3H/367/

var Model = can.Model.extend({

    define: {
        name: {
            value: 'Alex'            
        },
        '*': {
            serialize: false
        }
    }
})


var model = new Model({age: 10, name: 'John'})

console.log(model.serialize())

Why result is Object {name: "John"} ? I thought no props should be serialized, no? It is not very useful if * defines only not defnied attributes, I think it should aslo define each attribute properties.

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Apr 10, 2015

Contributor

@whitecolor Your use case is the entire reason why I wrote the define default behavior to begin with. I'll have a fix in before end of weekend.

Contributor

imjoshdean commented Apr 10, 2015

@whitecolor Your use case is the entire reason why I wrote the define default behavior to begin with. I'll have a fix in before end of weekend.

imjoshdean added a commit that referenced this issue Apr 12, 2015

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Apr 12, 2015

Contributor

@whitecolor - Check the provided diff. I added your test with a minor tweak to the define tests. Thanks for catching this, because as I said...your use case was the whole purpose to me making this feature. I want to tie AppState to can.route, but I don't want EVERYTHING put in AppState to be serialized.

Contributor

imjoshdean commented Apr 12, 2015

@whitecolor - Check the provided diff. I added your test with a minor tweak to the define tests. Thanks for catching this, because as I said...your use case was the whole purpose to me making this feature. I want to tie AppState to can.route, but I don't want EVERYTHING put in AppState to be serialized.

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Apr 12, 2015

Contributor

...aside from the linting erros. :S

Contributor

imjoshdean commented Apr 12, 2015

...aside from the linting erros. :S

@daffl daffl reopened this Apr 13, 2015

@daffl daffl modified the milestones: 2.2.5, 2.2.0 Apr 13, 2015

@daffl daffl closed this in #1618 Apr 14, 2015

daffl added a commit that referenced this issue Apr 14, 2015

Merge pull request #1618 from bitovi/1373_defaultDefineFix
Fix issue found in #1373 with not properly using defaults

imjoshdean added a commit that referenced this issue May 1, 2015

@imjoshdean imjoshdean removed their assignment Oct 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment