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

Component instances should implement getOwnKeys #326

Open
phillipskevin opened this issue Jan 8, 2019 · 7 comments
Open

Component instances should implement getOwnKeys #326

phillipskevin opened this issue Jan 8, 2019 · 7 comments

Comments

@phillipskevin
Copy link
Contributor

Right now if you create a component and call canReflect.getOwnKeys, you get a bunch of keys back:

import { Component, Reflect } from "//unpkg.com/can@5/core.mjs";

const C = Component.extend({
  tag: "a-pp",
  view: `
    Hello CanJS
  `,
  ViewModel: {}
});

const c = new C();

Reflect.getOwnKeys(c); // ["__inSetup", "_initialArgs", "element", "viewModel", "nodeList", "_torndown"]

... these aren't really useful.

Also, this causes CanJS Devtools to throw an exception because it tries to do something basically like this:

const v = new DefineMap({});
Reflect.updateDeep(v, { __inSetup: false });
@matthewp
Copy link
Contributor

matthewp commented Jan 8, 2019

what's the use-case for getOwnKeys exactly?

@phillipskevin
Copy link
Contributor Author

To get all of the keys of an object, even non-enumerable ones.

@matthewp
Copy link
Contributor

matthewp commented Jan 8, 2019

So that's what this is doing, right :)

@phillipskevin
Copy link
Contributor Author

Yeah, technically it is, but we do this same sort of override with other types (like DefineMap and DefineList) so that they don't return all of these keys that are added to instances by can-construct.

@phillipskevin
Copy link
Contributor Author

const m = new DefineMap();
Reflect.getOwnKeys(m); // []

I guess DefineList actually doesn't do this:

const l = new DefineList();
Reflect.getOwnKeys(l); // ["__inSetup", "_define", "_data", "constructor", "_length"]

for lists, devtools is only showing enumerable keys (for this same reason).

@matthewp
Copy link
Contributor

matthewp commented Jan 8, 2019

Ok, cool. I think we should update the ownKeys documentation to say that then (that objects can/should filter the list of returned values to be what you probably care about). My concern was that perhaps there was some usage of ownKeys that did expect all of the properties.

@phillipskevin
Copy link
Contributor Author

For now I'm going to fix the root cause (canjs/can-define#421), but I'll leave this open in case we decide it's worth doing at some point.

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