Skip to content

[WIP] Refactor Meta's deps structure to an array of arrays. #13659

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[WIP] Refactor Meta's deps structure to an array of arrays. #13659

wants to merge 2 commits into from

Conversation

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Jun 13, 2016 •
edited
Loading

@rwjblue
Copy link
Member Author

rwjblue commented Jun 13, 2016

@krisselden - I updated this to use an array of arrays (instead of a map of arrays). I left it in two commits so that it was easy to compare the difference (in case you end up liking the map of arrays approach better).

The best summary of the new structure is from writeDep where we setup the structure:

    // add the subkey to the outerArray
    keyArray.push([
      subkey,
      // array of itemKeys for the subkey
      [
        [itemKey, value]
      ]
    ]);

@rwjblue rwjblue changed the title [WIP] Refactor Meta's deps structure to a map of arrays. [WIP] Refactor Meta's deps structure to an array of arrays. Jun 13, 2016
@machty
Copy link
Contributor

machty commented Jun 13, 2016

this used to be 5.4 http://numberofpeoplewhounderstandembermetal.com/

chadhietala
chadhietala reviewed Jun 13, 2016
View reviewed changes
packages/ember-metal/lib/meta.js
let innerMap = outerMap[subkey];
if (!innerMap) {
innerMap = outerMap[subkey] = new EmptyObject();
Meta.prototype['write' + capitalized] = function(subkey, itemKey, value) {
Copy link
Contributor

@chadhietala chadhietala Jun 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can you do write${capitalized}, same goes for the other dynamic method. 😄

@rwjblue
Copy link
Member Author

rwjblue commented Jun 13, 2016

Updated #13660 to migrate to using the lodash-like object interface for this functionality. Closing this in favor of that.

@chadhietala - I'll make sure to incorporate your comments there though...

@rwjblue rwjblue closed this Jun 13, 2016
@rwjblue rwjblue deleted the meta-deps branch June 13, 2016 19:58
@rwjblue rwjblue mentioned this pull request Jun 13, 2016
Closed
@homu
Copy link
Contributor

homu commented Sep 25, 2016

☔ The latest upstream changes (presumably #13783) made this pull request unmergeable. Please resolve the merge conflicts.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rwjblue @machty @homu @chadhietala