Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Use search.$ in filter will result in Call stack exceeded error if the objects have properties that self reference #6655

Closed
pencilcheck opened this issue Mar 12, 2014 · 7 comments

Comments

@pencilcheck
Copy link

Create an object with properties that self-reference. Running a filter with search.$ as ng-model of an input will result in call stack exceeded fairly easily.

@caitp
Copy link
Contributor

caitp commented Mar 12, 2014

can you create an example?

The filter method should descend 50 levels deep if you have 50 properties on a single object, it will only recurse one property at a time, unless we're missing something here. If you post a reproduction we can investigate this.

@pencilcheck
Copy link
Author

@pencilcheck
Copy link
Author

Actually, it should be renamed as, self referencing objects

@caitp
Copy link
Contributor

caitp commented Mar 12, 2014

circular references don't work in the filter right now, there's a PR to prevent the error from occurring, but it's being left out for the time being due to the cost. It might be merged in one of the 1.3 betas. @IgorMinar what do you think about merging #6319 tentatively in the next beta? alternatively, the O(1) variation using the weakmap polyfill might also be worth doing

@pencilcheck
Copy link
Author

That sounds like a good news. Can you briefly explain the O(1) variation using the weakmap polyfill?

@caitp
Copy link
Contributor

caitp commented Mar 14, 2014

We looked at it again and decided the hash-key API in angular (used by ngRepeat) would be good enough, and won't need a weakmap --- so basically it's a collection of the objects we've already traversed, we ask the map if we've traversed something already, and if we have, it's circular so we ignore it.

You could come up with pathological cases where this doesn't work perfectly, and it is still possible to cause a stack overflow with deeply nested objects (deeper than the VM's stack depth). But it should mitigate that pain.

@Narretz
Copy link
Contributor

Narretz commented Feb 14, 2017

This is Won't fix / not core as explained in #6319 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants