search

ember-polyfills / ember-angle-bracket-invocation-polyfill

MIT License
76 stars 33 forks source link

Fix monkey patching in Ember 2.x polyfill code paths #125

Closed mmun closed 3 years ago

mmun commented 3 years ago

This PR fixes the bug identified in https://github.com/ember-polyfills/ember-angle-bracket-invocation-polyfill/issues/53. The fix in https://github.com/ember-polyfills/ember-angle-bracket-invocation-polyfill/pull/55 was only a partial fix: we also need to fix the monkey patching of manager.didCreateElement.

I noticed this because our CI (which has 1000s of tests) was crashing with a stack overflow.

The first commit in this PR contains a set of log statements that I added to demonstrate the problem. They're reverted in the second commit (and can be squashed out before merging). The third commit is the fix.

It's tricky to write a test for this that doesn't involve a very long test suite or even more monkey patching of an already complicated situation, so I opted to add some logging and to share some screenshots instead. The log statements are used to keep track of how deeply recursive the manager.didCreateElement calls get. You can see from the screenshots below that in Ember 2.12 and Ember 2.18 they grow linearly, but they should always be exactly one. The problem does not occur in Ember 3.3. After the fix is applied there are no recursive calls in any of the 3 versions I tested.

Before the fix (Ember 2.12):

Before - Ember 2 12 2

Before the fix (Ember 2.18):

Before - Ember 2 18 2

Before the fix (Ember 3.3):

Before - Ember 3 3 2

After the fix (Ember 2.12):

After - Ember 2 12 2

After the fix (Ember 2.18):

After - Ember 2 18 2
mmun commented 3 years ago

The CI failures seem unrelated to this PR.

mmun commented 3 years ago

I'm going to merge and release as is and I'm happy to follow up with any changes.