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

Allow someone to access keys #126

Closed
justinbmeyer opened this issue Dec 22, 2016 · 8 comments
Closed

Allow someone to access keys #126

justinbmeyer opened this issue Dec 22, 2016 · 8 comments

Comments

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Dec 22, 2016

Related to #125

I'm proposing a keys method on can-define that will work for any can-define type object:

var define = require("can-define");
var DefineMap = require("can-define/map/");
var DefineList = require("can-define/list/");

var Person = function(first, last) {
  this.first = first;
  this.last = last;
};
define(Person.prototype, {
  first: "*",
  last: "*",
  fullName: {
    get: function() {
      return this.first + " " + this.last;
    }
  }
});



var PersonMap = DefineMap.extend({
  first: "*",
  last: "*",
  fullName: {
    get: function() {
      return this.first + " " + this.last;
    }
  }
});

var PersonList = DefineList.extend({
  "#": Person,
  get count(){ return this.length }
});

var person = new Person("j","m");
var personMap = new PersonMap({first: "j", last: "m"});
var personList = new PersonList([ {first: "b", last: "m"} ]);

define.keys( person ) //-> ["first","last"]
define.keys( personMap ) //-> ["first","last"]
define.keys( personList ) //-> [0, "count"]

When reading keys, this will Observe.add(this, "__keys") ... things that add new keys should trigger this event.

@justinbmeyer
Copy link
Contributor Author

Part of me wonders if we should have our own symbol for this sort of thing. That way we can have generic can-util methods that use this sort of stuff.

@phillipskevin
Copy link
Contributor

@Bajix was looking for this a few days ago: https://gitter.im/canjs/canjs?at=5859bf21af6b364a29d9a102.

@Bajix
Copy link
Contributor

Bajix commented Dec 22, 2016

This should tie in w/ the work for adding define enumerable: #63

Also, here's a use case that we need to address:

  errors: {
    stream: function() {
      var errorStream = Kefir.merge([
        this.stream('.ajaxErrors'),
        this.stream('.validationErrors'),
        this.stream('.model.validationErrors')
          .filterBy(this.stream('._reqs'))
      ]).filter().skipDuplicates(deepEqual);

      return errorStream.flatMap(function( errors ) {
        var eventStream = this.stream('.model').flatMap(function( model ) {
          return Stream.toStreamFromCompute(compute(function() {
            return Object.keys(model.serialize());
          })).skipDuplicates(deepEqual).flatMap(function( __keys ) {
            var pool = Kefir.pool();

            __keys.forEach(function( key ) {
              pool.plug(Stream.toStreamFromProperty(model, key).skip(1).map(function( value ) {
                return {
                  type: key,
                  value: value
                };
              }));
            });

            return pool;
          });
        });

        return eventStream.scan(function( errors, ev ) {
          errors[ev.type] = undefined;
          return errors;
        }, errors);
      }.bind(this)).merge(Kefir.constant());
    }
  },

It's impossible right now to get a list of all keys that could be serialized. Above is an example of where that would've been extremely relevant

@justinbmeyer
Copy link
Contributor Author

@Bajix why is it impossible? Doesn't Object.keys(model.serialize()) work?

@Bajix
Copy link
Contributor

Bajix commented Dec 22, 2016

That's all keys that are serialized, not that could be serialized. The later would be necessary for simplifying the above use case

@justinbmeyer
Copy link
Contributor Author

justinbmeyer commented Dec 22, 2016

What is the difference between "are serialized" and "could be serialized"?

I'm guessing it's because we don't provide undefined values in the .serialize() result. Maybe that is a mistake?

@Bajix
Copy link
Contributor

Bajix commented Dec 22, 2016

What is the difference between "are serialized" and "could be serialized"?

All serializable keys with values set vs all properties on define that could be set.


I was using this for registration - the user password would initially be undefined, hence it wouldn't be part of Object.keys(model.serialize()). Once password is set, it doesn't emit a __keys event as it's already within the define definitions, so to observe it with streams I'd need to stream password directly, but there's no easy way of determining that it's a key that could be set at a later point. Serialize works because it reads definitions and bind to props that are initially undefined if they're serializable, plus __keys for unsealed maps.

The __keys event itself is broken. It should either include all keys of enumerable properties regardless of whether they're undefined and emit the __keys event whenever a new key comes into existence, or it should exclusively include keys of properties that are enumerable and defined. At the very least, __keys should be made streamable.

Maybe we could add something like DefineMap.prototype.definedKeys that takes an options object w/ enumerable & serialize settings and returns keys of properties matching, then make DefineMap.prototype.keys return enumerable keys based off the current instance rather than it's define definitions.

@phillipskevin
Copy link
Contributor

canReflect.getOwnEnumerableKeys is supported by can-define now.

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

3 participants