yapplabs / ember-buffered-proxy

An Ember Proxy the enables change buffering
MIT License
166 stars 31 forks source link

Utilize notifyPropertyChange instead of property(Will|Did)Change #37

Closed rondale-sc closed 6 years ago

rondale-sc commented 6 years ago

propertyWillChange and propertyDidChange are deprecated in Ember 3.1 and will be removed from the exports in Ember 3.5. Unfortunately notifyPropertyChange isn't available on the global until versions of Ember > 3.1. So the fix here is to use this.notifyPropertyChange(key) which should be available going back to 2.12 (which the test suites dummy app uses) and likely even earlier versions of Ember.

Please see deprecation link below for more details:

https://emberjs.com/deprecations/v3.x/#toc_use-notifypropertychange-instead-of-propertywillchange-and-propertydidchange

rondale-sc commented 6 years ago

@lukemelia These pass locally. I'm not sure if CI is configured differently now, but I did see a similar failure happen to a build a few months ago in the Build History that makes me think it might just need a rebuild. Any thoughts?

lukemelia commented 6 years ago

I'll kick it @rondale-sc

lukemelia commented 6 years ago

@rondale-sc No luck. Looks like a Travis config issue. Given the backward compat concerns with this PR, I think it's important that we get Travis green before proceeding. I don't have a time to look at this today, but may this weekend. If you or anyone else wants to tackle first, go for it, and let me know if you need anything.

rondale-sc commented 6 years ago

@lukemelia Totally agreed. I'll dive into this on Friday for at least a little bit and bump if I make progress.

rondale-sc commented 6 years ago

@lukemelia Issue with CI was related to this:

https://github.com/travis-ci/travis-ci/issues/8836#issuecomment-356362524

I followed the recommended workaround (set sudo: required in .travis.yml) and it seems to look good now.

lukemelia commented 6 years ago

@rondale-sc published as 1.0.0-beta.0. LMK if it works for you.

rondale-sc commented 6 years ago

@lukemelia Awesome! Thank you for being so quick to merge/release! :beers:

rondale-sc commented 6 years ago

@lukemelia Installed the beta, and it works like a charm. No more deprecation, and as far as I can tell no difference in behavior. Thanks for cutting the release!

Turbo87 commented 6 years ago

@lukemelia any plans on promoting this to stable? :)