🐞 Conditionally choose notifyPropertyChange context based on Ember version

[achambers]

✨ What Changed & Why

Somehow, Ember v3.13.x introduced a change that caused ember-buffered-proxy to stop working when calling notifyPropertyChange to update references to a buffered object.

As a result of this change, a "fix" was introduced to ember-buffered-proxy in 46d00d7 and 3f63beb which changed the way notifyPropertyChange was changed as follows:

- notifyPropertyChange(this, key);
+ notifyPropertyChange(content, key);

This seemed to fix the problem, although it doesn't conceptually make sense as to why (which is slightly worrying).

Fast forward to Ember v3.20.x and something else has changed in Ember source which has caused ember-buffered-proxy to stop working again. Or, more likely, whatever was introduced in Ember 3.13.x has been patched so that notifyPropertyChange, once again, requires this to be passed to it, as was originally the case.

This PR adds some version checking to ensure that notifyPropertyChange is called with the correct context depending on what version of Ember is being used.

I used this sandbox to verify which versions required which arguments so that I could specifically target the right Ember versions. The following is the table of versions I have focussed on:

Version	notifyPropertyChange(this)	notifyPropertyChange(content)
3.12	✅	❌
3.13	❌	✅
3.14	❌	✅
3.15	❌	✅
3.16	✅	✅
3.17	❌	✅
3.18	❌	✅
3.19	❌	✅
3.20	✅	❌
3.21	✅	❌
3.22	✅	❌
3.23	✅	❌
3.24	✅	❌

I have also verified that this fix works on the following versions: 3.12, 3.13, 3.19 and 3.20 so I am confident that this covers all possibilities.

Related Issues

Fixes #67

[achambers]

@lukemelia I've done a fair bit of manual testing of this to verify that the various versions now work with the single codebase. I did not, however, add an Ember Try scenario that would ensure that both the "broken" and "fixed" Ember versions work with this.

Is that something you'd prefer to see in this PR?

[achambers]

@lukemelia It seems the build failure is the same one that's happening on master currently (it's not tests, it's a yarn thing).

Any ideas?

[lukemelia]

I'll take a look at the build issues on master. Seems like it would be good to implement ember-try there.

Things are happening in #69

[achambers]

I'll take a look at the build issues on master. Seems like it would be good to implement ember-try there.

Things are happening in #69

Wicked. Let me know once that drops and I'll rebase this one, add an ember-try scenario that captures this PR logic and we should be good to go.

[lukemelia]

Looks like it has all passed there. I have merged. After you rebase, will you add one ember-try scenario for a failing ember version? (It so happens that all the LTS versions pass!)

[fsmanuel]

@achambers @lukemelia I'm also facing that issue. Thanks for fixing it. Would be great if we get the branch rebased and merged!

[lukemelia]

Thanks for your work on this @achambers. It has been incorporated into #87 by @fsmanuel and merged.