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

schedule watchTypeIfUnseen to prevent loop #8008

Merged
merged 9 commits into from
Jul 15, 2022
Merged

Conversation

patricklx
Copy link
Contributor

fixes #8006

@patricklx
Copy link
Contributor Author

could also fix emberjs/ember-inspector#1875

@kdagnan
Copy link

kdagnan commented Jun 17, 2022

@patricklx I was having a similar issue & stack trace to you - but I just tried running your branch locally and it hasn't fixed anything for me. Funny enough it's on a regex call. Do you think you could look at this one? Might be a quick/similar fix.
Uncaught (in promise) RangeError: Maximum call stack size exceeded at RegExp.exec (<anonymous>) at RegExp.test (<anonymous>) at isBlank (index.js:2729:1) at Object.isPresent (index.js:2768:1) at Store.modelFor (-private.js:10613:1) at Class.watchTypeIfUnseen (index.js:105:1) at store._createRecordData (index.js:69:1) at get _recordData [as _recordData] (-private.js:4989:1) at InternalModel._isRecordFullyDeleted (-private.js:5024:1) at InternalModel.isHiddenFromRecordArrays (-private.js:5018:1)

@patricklx
Copy link
Contributor Author

patricklx commented Jun 17, 2022

I think that's something else. You can get a Maximum call stack size exceeded with the regex.test alone as well if its on a strange string. At least i do not see a loop there

@snewcomer
Copy link
Contributor

@patricklx Would you mind adding a test if applicable to save-test.js?

@patricklx
Copy link
Contributor Author

@snewcomer i changed the debug-adapter-test to use createRecord. This will cause the test to fail, but work with the fix i provide.
since the update call is now delayed, it will also include the newly created record in the count, and no updates

@kdagnan
Copy link

kdagnan commented Jun 21, 2022

Can confirm this fix addressed similar loop issues for me. Originally it did not because I was running ember 3.28, ember-data 4.4 (your branch), and ember-cli 4.2.0. Updating ember-cli to 4.4.0 and manually adding your fix to the NPM-installed ember-data 3.28.10 fixed this issue.

Could we get this applied to a 3.28.11 as well? :)

@spruce
Copy link
Contributor

spruce commented Jul 3, 2022

This fixes #8006 for me

@runspired runspired added 🎯 canary PR is targeting canary (default) 🎯 lts The PR should be backported to the most recent LTS 🏷️ bug This PR primarily fixes a reported issue labels Jul 15, 2022
@runspired
Copy link
Contributor

@patricklx looks like the tests fail, mind rebasing and looking into that?

@runspired runspired merged commit 9d3ff54 into emberjs:master Jul 15, 2022
@brandensilva
Copy link

I would also appreciate a 3.28.11 release on this one. It had me scratching my head for half a day wondering how my typescript upgrades were call stacking and triggering illegal character issues. Thanks for fixing this @patricklx!

@patricklx patricklx deleted the patch-2 branch August 20, 2022 14:12
@runspired
Copy link
Contributor

@brandensilva in the works

runspired pushed a commit that referenced this pull request Sep 12, 2022
* schedule watchTypeIfUnseen to prevent loop

fixes #8006

* Update debug-adapter-test.js

* Update index.js

* Update index.js

* fix test

* Update debug-adapter-test.js

* keep Watching Model Types with store.push

* add comment

* fix test
runspired added a commit that referenced this pull request Sep 12, 2022
* backport fix for #7786 from #7882

* schedule watchTypeIfUnseen to prevent loop (#8008)

* schedule watchTypeIfUnseen to prevent loop

fixes #8006

* Update debug-adapter-test.js

* Update index.js

* Update index.js

* fix test

* Update debug-adapter-test.js

* keep Watching Model Types with store.push

* add comment

* fix test

* [BUGFIX] serialize null array items to string (#8083)

* port fix from #7834 for #7824

Co-authored-by: patricklx <patricklx@users.noreply.github.com>
Co-authored-by: Cameron Dubas <camerondubas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 canary PR is targeting canary (default) 🎯 lts The PR should be backported to the most recent LTS 🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

loop(max stack size exceeded) if creating an record, whose type was not loaded yet. and ember-inspector open
6 participants