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

Compact object #72

Closed
wants to merge 1 commit into from
Closed

Conversation

spencer516
Copy link
Contributor

This adds support to #64 for compacting objects.

FYI: Tests are failing on Beta/Canary due to this bug: emberjs/ember.js#12995

typeOf,
A: emberArray
} = Ember;
const keys = Object.keys || Ember.keys;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can kill Ember.keys, that is deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is; but doesn't 1.13 still support IE 8, which needs the Ember.keys polyfill?

@spencer516
Copy link
Contributor Author

^^ Did a bit of refactoring per your recommendations and got rid of that superfluous observer.

@homu
Copy link
Contributor

homu commented Feb 23, 2016

☔ The latest upstream changes (presumably #74) made this pull request unmergeable. Please resolve the merge conflicts.

@spencer516
Copy link
Contributor Author

I rebased this as it got a bit stagnant. If y'all have any other feedback, let me know.

@poteto
Copy link
Collaborator

poteto commented Mar 5, 2016

If you rebase this against master, beta will be an allowed failure.

} = Ember;
const keys = Object.keys || Ember.keys;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is not needed, just use Object.keys.

@homu
Copy link
Contributor

homu commented Mar 22, 2016

☔ The latest upstream changes (presumably #122) made this pull request unmergeable. Please resolve the merge conflicts.

JSCS Errors

Fixes error on Ember 1.13

Use isPresent

one liner

guard for non-arrays

Guard before setting the array

support object compacting

update readme

guard for non-array/non-object values

don't use array computed

be explicit about not supporting changing keys

don't use @each

remove unnecessary observer

use ember libs
@spencer516
Copy link
Contributor Author

Sorry I hadn't touched this in a while — must have missed that comment from 15 days ago.

@taras
Copy link
Contributor

taras commented Jul 27, 2016

Let's make this happen :)

Or:

```hbs
{{#each-in (compacy objectWithBlanks) as |notBlankKey notBlankValue|}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/compacy/compact/

@homu
Copy link
Contributor

homu commented Jul 30, 2016

☔ The latest upstream changes (presumably 4e9995f) made this pull request unmergeable. Please resolve the merge conflicts.

@ghost
Copy link

ghost commented Aug 23, 2016

We're not going to merge this, as compact cannot track object properties being added/removed, so it would lead to weird stuff happening updating the object after the helper has run once.

@ghost ghost closed this Aug 23, 2016
This pull request was closed.
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