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

can.Map.prototype.define #819

Closed
justinbmeyer opened this issue Mar 23, 2014 · 18 comments
Closed

can.Map.prototype.define #819

justinbmeyer opened this issue Mar 23, 2014 · 18 comments

Comments

@justinbmeyer
Copy link
Contributor

I'm proposing a can/map/define plugin that would clean up several issues with the can/map/setter and can/map/attributes plugin. It will be a replacement for those plugins.

Example

Issues to think about:

Issue this should fix:

Issue this might fix:

can.Map.prototype.define

@Property {Object<String,can.Map.attrDefinition>} can.Map.prototype.define

Defines a map attributes setter, type, and remove functionality and default values.

Use

A can.Map's define property is used to define behavior for attributes of instance of the can.Map. The following defines a make-model-year view model that:

  • on initialization loads a list of makes.
  • on makeId being set, loads models for that make, removes modelId.
  • on modelId being set, loads years for that model, removes year.
var MakeModelYear = can.Map.extend({
    define:{
        makes: {
            value: function(){
                CarMake.findAll({}, function(carMakes){
                    self.attr("makes", carMakes);
                });
            }
        },
        makeId: {
            type: "number",
            set: function(makeId){
                this.removeAttr("models");
                var self = this;
                CarModel.findAll({makeId: makeId}, function(carModels){
                    self.attr("models", carModels)
                });
                return makeId;
            },
            remove: function(){
                this.removeAttr("models")
            }
        },
        models: {
            Type: CarModel.List,
            remove: function(){
                this.removeAttr("modelId")
            }
        },
        modelId: {
            type: "number",
            set: function(modelId){
                this.removeAttr("years");
                var self = this;
                Year.findAll({modelId: modelId}, function(carYears){
                    self.attr("years", carYears)
                });
                return makeId;
            },
            remove: function(){
                this.removeAttr("years")
            }
        },
        years: {
            Type: Year.List,
            remove: function(){
                this.removeAttr("year")
            }
        },
        year: {
            type: "number"
        }
    }
});

can.Map.prototype.attrDefinition

@typedef {{set: can.Map.defineSetter, remove: can.Map.defineRemover, value: *|can.Map.defineDefaulter, type: can.Map.defineConverter | can.Map.defineType, Type: function}} can.Map.prototype.attrDefinition

Defines a map attributes setter, type, and remove functionality and default values.

@option {can.Map.defineConverter | can.Map.defineType} type A function that converts the
value to a specified type. Example:

@option {function} Type A constructor function that takes the value as the first argument and called with new.

can.Map.defineSetter

@function can.Map.defineSetter

@Signature setter( newVal, setValue )

@return {*} If a non undefined value is returned, the value returned is set on the map.

@Body

Use

The following shows an async setter:

define: {
  prop: {
    set: function( newVal, setVal){
      $.get("/something", {}, setVal );
    }
  }
}

can.Map.defineRemover

@function can.Map.defineRemover

Called when an attribute is removed.

@Signature remover( currentValue )

@return {*|false} If false is returned, the value is not removed.

@Body

Use

The following prevents removing the prop attribute if someone tries to remove the value 0:

define: {
  prop: {
    remove: function( currentVal ){
      return currentVal !== 0;
    }
  }
}

can.Map.defineDefaulter

@function can.Map.defineDefaulter

Returns the default value for instances of this can.Map. This is called before init.

@Signature defaulter()

@return {*} The default value. This will be passed through setter and type.

can.Map.defineType

@function can.Map.defineType

A function that converts what's returned by the setter to some type.

@mjstahl
Copy link
Contributor

mjstahl commented Mar 23, 2014

From my limited testing the setter is not getting called when removeAttr is
called.

The event is being thrown, but no setter is being called.
On Mar 23, 2014 3:08 PM, "Justin Meyer" notifications@github.com wrote:

I'd like to add the ability to hook into removing an attribute like one
can hook into setAttr.

@mjstahl https://github.com/mjstahl Did you figure out if this was
already happening?

We can either make .removeAttr call the setATTR with undefined, or have
another .removeATTR method like:

setContentId: funciton(newVal){
return newVal
},
removeContentId: function(){

}

We probably want a way to prevent removing a property. I suggest returning
any value will prevent removal.

Thoughts?

Reply to this email directly or view it on GitHubhttps://github.com//issues/819
.

@mjstahl
Copy link
Contributor

mjstahl commented Mar 23, 2014

I like the removeATTR functionality.

I would also propose that we pass in the value that the attribute was at
the time of removal.

It should be noted that I do not have a use case for accepting the value of
the removed attribute as an argument.
On Mar 23, 2014 3:08 PM, "Justin Meyer" notifications@github.com wrote:

I'd like to add the ability to hook into removing an attribute like one
can hook into setAttr.

@mjstahl https://github.com/mjstahl Did you figure out if this was
already happening?

We can either make .removeAttr call the setATTR with undefined, or have
another .removeATTR method like:

setContentId: funciton(newVal){
return newVal
},
removeContentId: function(){

}

We probably want a way to prevent removing a property. I suggest returning
any value will prevent removal.

Thoughts?

Reply to this email directly or view it on GitHubhttps://github.com//issues/819
.

@justinbmeyer justinbmeyer changed the title removeAttr hooks can.Map.prototype.define Mar 24, 2014
@moschel
Copy link
Contributor

moschel commented Mar 24, 2014

+1 looks like a good, clean API

there was overlap between setter and attributes, so this is clearer

@matthewp
Copy link
Contributor

What does type do? Does it reject values not of that type or does it attempt to coerce the value to the correct type?

@moschel
Copy link
Contributor

moschel commented Mar 28, 2014

As discussed in the hangout today, this will be added to can.List as well.

Adding it here to make sure it doesn't get lost.

@cherifGsoul
Copy link
Member

I see it as a clean API for defining business logic services I like it

@justinbmeyer justinbmeyer added this to the 2.1.0 milestone Apr 3, 2014
@justinbmeyer justinbmeyer self-assigned this Apr 3, 2014
@wclr
Copy link
Contributor

wclr commented Apr 4, 2014

convert (i think what is "type" for?) and serialize should be added as well.

@justinbmeyer
Copy link
Contributor Author

Yes type is for convert, but b/c you can also specify Type, I think type makes sense. Serialize should be added. Thanks!

@moschel
Copy link
Contributor

moschel commented Apr 4, 2014

@justinbmeyer @whitecolor do you mean an equivalent for http://canjs.com/docs/can.Map.attributes.static.serialize.html, but defined for each attribute?

@wclr
Copy link
Contributor

wclr commented Apr 4, 2014

yes, and as api propsal serialize: false should instruct serializer to not touch the property at all.

@moschel
Copy link
Contributor

moschel commented Apr 4, 2014

I like that idea. Makes sense for things like a can.List of objects that will be represented in its serialized form elsewhere by a list of Ids. For example (similar to #752 (comment)):

locations: {
  serialize: false
  set: function(val){

  },
},
locationIds: {
  get: function(){
    return this.attr('locations').filter(function(location){
                return this.location.attr('selected');
            });
  },
  serialize: function(locationIds){  return this.join(','); }
}

@wclr
Copy link
Contributor

wclr commented Apr 5, 2014

Are you going to add here plain object/array attributes support (that won't be converted to can.Map)?

@wclr
Copy link
Contributor

wclr commented Apr 16, 2014

Another feature that could be implemented in this plugin, is to allow out of the box setting deferred values to attributes

// so this would wait for findAll result and assign it to "items"
app.attr('items', Item.findAll)

for this in define there could be defined some property

define: {
    items: {
        acceptDeferred: true, //or smth like that
    }
}

@justinbmeyer
Copy link
Contributor Author

You could do this within set yourself:

set: function(newVal, setVal){
  if( can.isDeferred(newVal) ) {
    newVal.then(setVal)
  } else {
    setVal(newVal);
  }
})

@justinbmeyer
Copy link
Contributor Author

Are you going to add here plain object/array attributes support (that won't be converted to can.Map)?

Yes, you do that like:

define: {
  plain: { type: "*" }
}

@wclr
Copy link
Contributor

wclr commented Apr 16, 2014

Yes, I understand I meant maybe to support setting deferreds out of the box. is the plugin ready for use/test?

@moschel
Copy link
Contributor

moschel commented May 1, 2014

this has been merged into minor

@wclr
Copy link
Contributor

wclr commented May 1, 2014

It seems to be a wrong context inside serialize function ("this" is window object)

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

No branches or pull requests

6 participants