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

[optimization] Performance and DX fixes #522

Merged
merged 1 commit into from
May 16, 2018

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented May 16, 2018

  • Ensures that we are recycling td components
  • Removes ObjectProxies in favor of using meta values directly
  • Adds tests for meta chaches
  • Fixes a bug where two tables rendering at the same time can cause
    a rerender by sorting the columns twice
  • Adds tests for selection mode with tree. Tree selection is going to
    need to be merged into the CollapseTree, one test has been skipped.
  • Adds some simple scenario pages that work on 1.11+ so we can go in and
    do perf testing, etc.
  • Enhances generator functions so they can generate trees as well

* Ensures that we are recycling `td` components
* Removes ObjectProxies in favor of using meta values directly
* Adds tests for meta chaches
* Fixes a bug where two tables rendering at the same time can cause
  a rerender by sorting the columns twice
* Adds tests for selection mode with tree. Tree selection is going to
  need to be merged into the CollapseTree, one test has been skipped.
* Adds some simple scenario pages that work on 1.11+ so we can go in and
  do perf testing, etc.
* Enhances generator functions so they can generate trees as well
@mixonic
Copy link
Member

mixonic commented May 16, 2018

@pzuraq the API for meta is that you are always making metas the last argument for a yield? Seems good.

The other approach would be to use a weakmap polyfill in IE11 that pollutes the object with a non-enumerable property, and a native weakmap polyfill in modern browsers.

But I really like this cleanup. It is much easier to understand what is happening without the proxy. 👍

Note for later: I wonder why we need so many emberA( calls. If we're regenerating arrays instead of mutating them, then we should be fine to use native arrays and skip on .[] dependency keys.

@pzuraq pzuraq merged commit da352e4 into master May 16, 2018
@pzuraq pzuraq deleted the pzuraq/optimization/performance-and-dx branch May 16, 2018 17:14
@pzuraq
Copy link
Contributor Author

pzuraq commented May 16, 2018

The other approach would be to use a weakmap polyfill in IE11 that pollutes the object with a non-enumerable property, and a native weakmap polyfill in modern browsers.

We can actually use WeakMap in IE11, and we are using WeakMaps to store and retrieve the meta objects as we scroll. The reason we yield them is so that users of the table can access the meta objects to put meta information on them.

If we made the only way to access meta through the weakmap, we would have to expose a public method for retrieving meta (e.g. tableMetaFor(row)), and this would be more complicated for cell meta's since they don't actually exist. Cell values are row[column.valuePath], so to get a unique meta for a cell users would have to do tableMetaFor(row, column) or something like that. It would also make binding to meta values in templates very tricky, and that's definitely something we want to support.

I wonder why we need so many emberA( calls. If we're regenerating arrays instead of mutating them, then we should be fine to use native arrays and skip on .[] dependency keys.

This is purely for backwards compatibility - 1.11 (and 1.13 I believe) assert that arrays passed to {{each}} are Ember Arrays.

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

Successfully merging this pull request may close these issues.

2 participants