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

#54 introduces behavior difference depending on Ember version #76

Closed
SaladFork opened this issue Aug 3, 2016 · 7 comments
Closed

#54 introduces behavior difference depending on Ember version #76

SaladFork opened this issue Aug 3, 2016 · 7 comments

Comments

@SaladFork
Copy link

In #54, a shim was added for ember-platform/assign which will use Ember.assign if available (≥ 2.5.0) and Ember.merge otherwise. These do not have the same method footprint/behavior so could be a concern.

Ember.assign({}, {foo: 3}, {bar: 5})
// -> Object {foo: 3, bar: 5}

Ember.merge({}, {foo: 3}, {bar: 5})
// -> Object {foo: 3}
@rwjblue
Copy link
Member

rwjblue commented Aug 3, 2016

yeah, the issue is that we already used the named export of assign. Leaving import { assign } from 'ember-platform'; as always stuck to be a two arg situation is pretty crappy.

Do you have a specific scenario that breaks?

@SaladFork
Copy link
Author

SaladFork commented Aug 3, 2016

No, and I don't think it's too big a problem. Unless I'm mistaken you'd have to be downgrading Ember to run into this, as you'd have to be using assign with >2 arguments and going to an environment where Ember.assign is no longer available.

@rwjblue
Copy link
Member

rwjblue commented Aug 3, 2016

Right. One option I had thought of was to just include the shim directly so that assign always "just worked" with arbitrary args. But we generally don't want this repo to include "code"...

@john-kurkowski
Copy link

It'd be a breaking change to this lib, but would it otherwise hurt to export both?

They're not perfectly interchangeable. When Ember added it, they didn't claim assign to be a replacement. Neither is deprecated.

@rwjblue
Copy link
Member

rwjblue commented Aug 27, 2016

FWIW, Ember.merge was definitely deprecated when we added the Ember.assign feature. There were many issues with the deprecation being extremely noisy in the ecosystem, so we disabled the deprecation for 2.5.1 (see CHANGELOG.md or PR disabling the deprecation).

@john-kurkowski
Copy link

john-kurkowski commented Aug 27, 2016

Ah, interesting. I only looked at the latest API docs. Thank you for the clarification.

@Turbo87
Copy link
Member

Turbo87 commented Nov 13, 2017

closing due to inactivity

@Turbo87 Turbo87 closed this as completed Nov 13, 2017
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

No branches or pull requests

4 participants