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

[BUGFIX] Fix computed sort regression when array prop initially null #15733

Merged
merged 1 commit into from
Oct 13, 2017

Conversation

ilucin
Copy link
Contributor

@ilucin ilucin commented Oct 12, 2017

There was a regression introduced (commit) in a sort computed macro.

If an array property (that is being sorted) is initially null or other non-array value - when computed property is first evaluated observers wouldn't be set and computed property would never work again. Provided test pretty much says it all.

@bekzod
Copy link
Contributor

bekzod commented Oct 13, 2017

was my fault 🤕, was about to submit PR for this, good job @ilucin 👍

@bekzod
Copy link
Contributor

bekzod commented Oct 13, 2017

fixes #15732

@rwjblue rwjblue merged commit 032a271 into emberjs:master Oct 13, 2017
@rwjblue
Copy link
Member

rwjblue commented Oct 13, 2017

Thank you!

@ilucin
Copy link
Contributor Author

ilucin commented Oct 16, 2017

@rwjblue No probs. Shouldn't we release 2.16.1 with this fix included?

@Techn1x
Copy link

Techn1x commented Oct 16, 2017

We're already up to 2.16.2 (I think?), so I guess it would be 2.16.3? I'm waiting for this PR fix before upgrading, since the broken sort does break some functionality in my webapp :/

@Techn1x
Copy link

Techn1x commented Oct 18, 2017

According to this;
8bfa201

It looks like the fix is in 2.17-beta.2. Seems like it's going to be a few weeks before the fix is in stable channel, if it's targeted at 2.17...

@ilucin
Copy link
Contributor Author

ilucin commented Oct 18, 2017

I've opened another PR #15746 (over release branch) for 2.16.1

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