Addepar / ember-table

https://opensource.addepar.com/ember-table/
Other
1.7k stars 351 forks source link

Feeding RecordArray from ember-data causes an error #584

Open vihai opened 6 years ago

vihai commented 6 years ago

First of all I wish to thank you for the good work.

I'm using Ember 3.0.0 with git's master of ember-table

I'm setting up a new table whose rows are fed from an ember-data RecordArray and I'm getting this javascript error, even when the RecordArray is actually empty:

collapse-tree.js:356 Uncaught TypeError: Ember.get(...).some is not a function
    at CollapseTreeNode.get isLeaf (collapse-tree.js:356)
    at ComputedProperty.get (ember-metal.js:2536)
    at Object._get [as get] (ember-metal.js:1358)
    at CollapseTreeNode.get length (collapse-tree.js:448)
    at ComputedProperty.get (ember-metal.js:2536)
    at _get (ember-metal.js:1358)
    at _getPath (ember-metal.js:1405)
    at Object._get [as get] (ember-metal.js:1390)
    at CollapseTree.get length (collapse-tree.js:626)
    at ComputedProperty.get (ember-metal.js:2536)

Explicitly disabling tree with enableTree=false property works around the problem.

pzuraq commented 6 years ago

We’re using actual array functions when RecordArray and in general Ember arrys don’t support them. Always tricky 😩

Thanks for reporting, should be an easy fix!

vihai commented 6 years ago

I'm having a hard time trying to use a custom object as rows source. My plan would be to implement an infinite-scrolling source with dynamic paged retrieval of objects, however the following line is checking that rows is an array which it isn't:

assert('value must have an array of children', isArray(get(value, 'children')));

Removing the assertion trigger another assertion elsewhere:

Error: Assertion Failed: index must be gte than 0

You may want do a comprehensive check that a non-array rows object can be used.

It would probably be nice to know whether a row is not visible anymore to eventually unload the model.

Also it would be very convenient to be able to not provide a length in advance, it would very much simplify the backend; however I don't know if it would be feasible.

I'm giving up for now but I'll monitor this bug.

pzuraq commented 6 years ago

The assertion is using Ember.isArray which should return true for any class which is an actual array, or an EmberArray. I suppose we could be less strict here, and check only that the passed in value has length and objectAt methods (fulfills the array interface) but that may require us rewriting some of the internals.

jamesdixon commented 6 years ago

@pzuraq curious if they plan on adding some of the standard array methods to Ember arrays. Seems like it makes sense from a consistency perspective. For now I'm just converting to regular arrays via toArray()

pzuraq commented 6 years ago

I think it's the opposite, in general they're probably going to be moving most public things away from Ember arrays and toward native arrays. I'm not so sure what'll be going on with Ember Data though.

Frozenfire92 commented 5 years ago

Just ran into this today,

what I did to maintain the benefits of the RecordArray live updating is have an intermediate computed for my rows

rows: computed('recordArray.[]', function() {
  return get(this, 'recordArray').toArray();
});
BryanCrotaz commented 2 years ago

This is still a bug in 2022