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

Bug: Ember.get with empty string as path no longer returns target object #14572

Closed
jasonmit opened this issue Nov 2, 2016 · 5 comments
Closed

Comments

@jasonmit
Copy link
Member

jasonmit commented Nov 2, 2016

Previously, Ember.get(obj, '') would return obj.

This behavior was removed in #14345 and caused some breakage in ember-cp-validations which relied on this. adopted-ember-addons/ember-cp-validations#380

My opinion is this was likely a bug that people relied on that could instead be marked for deprecation.

@jasonmit jasonmit changed the title Ember.get with empty string as path no longer returns target object Bug: Ember.get with empty string as path no longer returns target object Nov 2, 2016
@Dhaulagiri
Copy link
Contributor

@jasonmit see #14386 for discussion of a similar issue related to the change from #14345

@jasonmit
Copy link
Member Author

jasonmit commented Nov 3, 2016

Thanks, I'll close this one

@jasonmit jasonmit closed this as completed Nov 3, 2016
@rwjblue
Copy link
Member

rwjblue commented Nov 3, 2016

I still think we can make an assertion that the key passed to Ember.get is not empty string. That would at least have made the other logic issue (in Ember-count-validations) easier to track down.

Serabe added a commit to Serabe/ember.js that referenced this issue Nov 4, 2016
obj`

This adds an assertion to let the user know when they are using an empty
string with an object that has no `''` property.

I'm keeping `Ember.get({'': 42}, '')` as a valid expression given it is
a tested behaviour.

Addresses [this
comment](emberjs#14572 (comment)) in emberjs#14572
Serabe added a commit to Serabe/ember.js that referenced this issue Nov 4, 2016
This adds an assertion to let the user know when they are using an empty
string as the key in `Ember.get`.

Addresses [this
comment](emberjs#14572 (comment)) in emberjs#14572
stefanpenner pushed a commit to chadhietala/ember.js that referenced this issue Nov 4, 2016
This adds an assertion to let the user know when they are using an empty
string as the key in `Ember.get`.

Addresses [this
comment](emberjs#14572 (comment)) in emberjs#14572
rwjblue pushed a commit that referenced this issue Nov 7, 2016
This adds an assertion to let the user know when they are using an empty
string as the key in `Ember.get`.

Addresses [this
comment](#14572 (comment)) in #14572

(cherry picked from commit d03ff1f)
@technomage
Copy link

Note that this is a breaking change, and should have been a deprecation (in my opinion), not cause working tests to fail when migrating from 2.7 to 2.10 and requiring patch to add ons relying on the prior behavior. In my case an add-on ember-cli-select-picker.

@bstro
Copy link

bstro commented Mar 29, 2018

This is breaking a bunch of our tests – is there any particular trick to finding the call site where Ember.get is being called with an empty string? This error is occurring when an acceptance test tries to render a handlebars template and my stack-trace has no helpful information. Not sure how else I can debug this.

update:
found the cause but not the reason: in the test, we were rendering a component like so:

      this.render(hbs`{{milestones/assessment-plot
          plotStyle="overview"
          assessmentsForThisGraph=[]
          rubric=academicRubric
          showStandards=false
        }}
      `);

This produced a render call to HTMLBars like such:

this.render(Ember.HTMLBars.template({
        'id': 'Q8LZjo7P',
        'block': '{"statements":[["append",["helper",["milestones/assessment-plot"],null,[["plotStyle","assessmentsForThisGraph","rubric","showStandards"],["overview",["get",[""]],["get",["academicRubric"]],false]]],false],["text","\\n      "]],"locals":[],"named":[],"yields":[],"blocks":[],"hasPartials":false}',
        'meta': {}
      }));
      (0, _chai.expect)(this.$('[data-test-nodata-add-assessment]')).to.not.exist;

Notice the ["overview",["get",[""]]… in the above. That get instruction seems to be getting called with an empty string. On a hunch, I replaced assessmentsForThisGraph=[] in the render call with assessmentsForThisGraph=(array). This fixed the tests, but it seems like I shouldn't have to use a helper from a third-party library to get the tests to run?

I'm going with this fix for now but would love to know more if anyone has ideas. also, I don't see this anywhere in deprecation notes, but it really ought to be mentioned somewhere, if it isn't.

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

5 participants