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

fix ember-scheduler to work with native class syntax #60

Merged
merged 3 commits into from
Jan 22, 2019

Conversation

toovy
Copy link

@toovy toovy commented Dec 7, 2018

Using https://github.com/pzuraq/ember-native-class-polyfill and probably ember 3.6 there is an issue with the native class syntax and computed properties. A similar issue occurred in ember-concurrency (see machty/ember-concurrency#261) and will be fixed in machty/ember-concurrency#262.

I've changed the ember-scheduler to now work with the class based syntax for the TaskProperty. As the test are still running I assume that the fix works.

@bistin
Copy link

bistin commented Dec 17, 2018

It seems missing

if (super.setup) {
     super.setup(...arguments);
}

in the setup function like https://github.com/machty/ember-concurrency/blob/master/addon/-task-property.js#L439-L442

After adding these code to setup function, it works in my project running in ember 3.6

@cibernox
Copy link
Collaborator

I'm also experiencing this when I update to Ember 3.6

@toovy
Copy link
Author

toovy commented Dec 19, 2018

I've added the conditional super call and updated this pull request. The CI still fails because of a dependency. I'll check that.

@toovy
Copy link
Author

toovy commented Dec 19, 2018

While ember-animated runs agains 3.6 in my simple test scenario the checks for ember try:one with ember-release ember-beta and ember-canary fail. I think that most of the issues are not related to ember-animated, e.g. willTransition is not used anywhere, maybe some test dependencies need to be updated. Any help is appreciated.

@cibernox
Copy link
Collaborator

cibernox commented Dec 19, 2018

@toovy This PR I just did on ember-app-scheduler should fix your problems or at least some of them: ember-app-scheduler/ember-app-scheduler#192

@cibernox
Copy link
Collaborator

@toovy I believe that if you now delete yarn.lock and reinstall dependencies it should work.

@toovy
Copy link
Author

toovy commented Dec 20, 2018

@cibernox yes, removing the yarn.lock and reinstalling worked, thanks. So I guess we have to wait will the PR you made is merged. There is still the issue with Uncaught TypeError: Ember.Test.adapter.exception is not a function that seems to be outside ember-animated and also a perform issue that points to ember-concurrency.

@cibernox
Copy link
Collaborator

@toovy I turned out my PR wasn't needed, someone did it already. I closed it. Whatever is failing now is unrelated.

@ef4 ef4 merged commit e301013 into ember-animation:master Jan 22, 2019
@ef4
Copy link
Contributor

ef4 commented Jan 22, 2019

Thanks for helping with this.

@toovy
Copy link
Author

toovy commented Jan 22, 2019

np 👍

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

Successfully merging this pull request may close these issues.

4 participants