ciena-frost / ember-frost-core

http://ciena-frost.github.io/ember-frost-core/
MIT License
18 stars 42 forks source link

Add object helper #531

Closed EWhite613 closed 6 years ago

EWhite613 commented 6 years ago

This project uses semver, please check the scope of this pr:

CHANGELOG

EWhite613 commented 6 years ago

@notmessenger there's no case for this helper in ember-frost-core's dummy app

As for overriding the hash helper I don't think we can do that as it's not a regiestered Ember helper. Even if you create a hash helper it's still going to use the glimmer hash helper.

~Plus are we going to 2.13.x? I expect our next jump is 2.16/17.x which looks like it still has the issue https://github.com/emberjs/ember.js/blob/v2.16.2/packages/ember-glimmer/lib/helpers/hash.js#L30-L34 (https://github.com/emberjs/ember.js/blob/v2.16.2/packages/ember-metal/lib/property_set.js#L46)~

Edit: nvm looks like they just fixed the Array's to String instead ( https://github.com/emberjs/ember.js/blob/master/packages/ember-utils/tests/to-string-test.js#L24)

I'll see if I can add that fix in (fixing ember-util toString). Don't know if I'll have access to that tho

notmessenger commented 6 years ago

@EWhite613 correct, I was going to point out that the fix was https://github.com/emberjs/ember.js/pull/15117 and was merged into both 2.13.0 and #lts-2-12 (incorrectly) and that the fix remains all the way to the current versions.

notmessenger commented 6 years ago

Even if we can't extend the Ember-provided hash helper are we not able to register/provide our own of the same name that gets consumed instead? I think I did something like that back in the day but I may be misremembering as well.

EDIT: may have to do via an initializer

notmessenger commented 6 years ago

👍

Approved with PullApprove