CirclonGroup / angular-tree-component

A simple yet powerful tree component for Angular (>=2)
https://angular2-tree.readme.io/docs
MIT License
1.1k stars 494 forks source link

Cannot assign to read only property 'id' of object '#<Object> #220

Closed yaronmil closed 7 years ago

yaronmil commented 7 years ago

i dont know if it is a bug but when using ngrx store as data for tree nodes it throws this exception.

adamkleingit commented 7 years ago

You need to specify a unique ID property to all nodes, otherwise the tree tries to assign one itself

--

On Tue, Mar 14, 2017 at 5:10 PM, yaronmil notifications@github.com wrote:

i dont know if it is a bug but when using ngrx store as data for tree nodes it throws this exception.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/500tech/angular-tree-component/issues/220, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2SSpBQxXu4JcLKqtkweLPyiysf4VEgks5rlq34gaJpZM4MctfI .

yaronmil commented 7 years ago

@adamkleingit my error is at tree-node.model.js (line 134 at my code version)

 TreeNode.prototype.setField = function (key, value) {
        this.data[this.options[key + "Field"]] = value;
    };

it is trying to assign a value to the node object. the node object is protected from changes (iam using redux store) . all node properties are set to readonly. the exception thrown is Cannot assign to read only property 'id' of object '#<Object>'

dojchek commented 7 years ago

@yaronmil I'm on 3.2.3 version and I'm also using this component with ngrx and managed to get the nodes and selected node working with unidirectional dataflow - well, to some extent..

You should keep your nodes normalized in the store and then denormalize it in the selector which you use to provide the nodes to tree component. Every time the nodes state changes in the store, the new denormalized collection should be created and it should work just fine. How exactly do you populate the nodes input?

yaronmil commented 7 years ago

@dojchek thank you, i now denormalize the nodes in the selector and it seems to work ok ! i was not thinking of doing the nodes store manipulation in the selector.

sarora2073 commented 7 years ago

Hi,

I'm using v3.3.1 (latest) and am experiencing the same error Cannot assign to read only property 'id' of object '#'. I tried changing the idField property to '_id' and get the slighly modified error 'Cannot assign to read only property '_id', suggesting to me that the idField option was set correctly but doesn't work as intended.

I'm pulling the data as an observable, so need this to work with immutable data. As a test, i commented out the line shown below in tree-node.model.js and that "solved" the problem but not sure if it creates unwanted side-effects. Any help appreciated. - S. Arora

TreeNode.prototype.setField = function (key, value) {
    // this.data[this.options[key + "Field"]] = value;
};

p.s. can anyone re-open this one? Or am I supposed to open a new issue? Also, i don't follow the workaround above about "denormalizing the selector"...not sure if that makes sense in my case as i'm not using ngrx...

KeithGillette commented 7 years ago

@sarora2073, I just encountered the same error with a node list property assigned from an Observable. The solution I found was to use Object.assign to create fresh copies of each node in the onNext function passed in the subscription to the Observable data source:

node-list.component.ts

public onInit() {
    this.nodeListReadAndSubscribe();
}

public nodeList: INode[] = [];

private nodeListReadAndSubscribe(): void {

    const onNext = (resultList: ObservableQueryResult<INodeListQueryResponseData>): void => {
        this.nodeList = [];
        resultList.forEach((node: INode) => {
            this.nodeList.push(Object.assign({}, node));
        });
    };

    const onError = (error: string): void => {
        // @ToDo handle the error
    };

    const onComplete = (): void => {
        // @ToDo: clean up
    };

    this.nodeListReadQuerySubscription = this.observableDataService.nodeListQuery()
        .subscribe(onNext, onError, onComplete);

}
sarora2073 commented 7 years ago

Hi,

Thanks for sharing that. My final solution was similar...

  1. import lodash import * as _ from 'lodash';
  2. create nodes array (mutable data structure) nodes: any[];
  3. do a http.get to retrieve the observable data, then in .subscribe section populate my mutable data structure with the results like so: return this.http.get('/getData') .map(data => data.json()) .subscribe((res: any[]) => { this.nodes = _.cloneDeep(res); }, error => this.handleError(error));

The solution above won't allow you to pull updates to the package, so figured you might like this approach better...

S. Arora

mikedobrin commented 7 years ago

tree-node.model.js line 20

this.id = (this.id !== undefined && this.id !== null) ? this.id : uuid(); // Make sure there's a unique ID

should be changed to

if (this.id === undefined || this.id === null) {
    this.id = uuid();
}

that way we will prevent setField from being called on immutable objects when 'id' is provided

mikedobrin commented 7 years ago

@yaronmil, should the ticket be reopened?

KeithGillette commented 7 years ago

@mikedobrin, Since the current behavior seems to require a work-around of deep copying to display a hierarchy of immutable objects, it would be great if the angular-tree-component code were changed to respect the existing id on immutable data structures. I take it that the code changes to line 20 of tree-node.model.js have not been made in subsequent releases since this issue was reported?

cristianpoleyJS commented 7 years ago

Same issue:

ERROR TypeError: Cannot assign to read only property 'checked' of object '[object Object]'

sarora2073 commented 7 years ago

@cripolgon - without looking at your code I'm not sure if you're running into a real issue or not, but I would consider the following:

  1. Data structures (e.g. data result from an observable stream) can be immutable by-design
  2. The original issue here was that this package was trying to modify the immutable data structure, so as a workaround we cloned the immutable data structure and provided a mutable data structure instead
  3. The link to #326 suggests this was solved, so we should now be able to pass an immutable data structure

Given all this, when I look at the error you mentioned, i'm wondering if you also are trying to modify an immutable data structure? If so, you may be getting a "normal" error. If I've missed your scenario entirely, I'd be glad to take a look if you want to post some code or a plnkr etc.

Regards,

  • S. Arora