ember.js: each-in breaks on keys that contain a period in 3.8.0-beta.1

{{each-in}} will return undefined for the value whose key contains a period.

This bug does not exist in 3.7.2 and first appears in 3.8.0-beta.1 and is still present in 3.8.0-beta.3.

Fail test (drop in packages/@ember/-internals/glimmer/tests/integration/syntax/each-in-test.js)

[`@only each-in supports keys with a period in them`]() {
  this.render(
    strip`
    <ul>
      {{#each-in categories as |_ item|}}
        <li>{{item.name}}</li>
      {{/each-in}}
    </ul>
  `,
    {
      categories: {
        // uncomment and run.  notice `items` is undefined
        'hello.world': { name: 'foo' },
        // uncomment and run.  notice it works as expected
        // hello world: { name: 'foo' },
      },
    }
  );

  // Empty
  this.assertHTML(strip`
    <ul>
      <li>foo</li>
    </ul>
  `);
}

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 2
Comments: 25 (19 by maintainers)

Commits related to this issue

[BUGFIX #17529] Refactor get to return object or path — committed to eplata31/ember.js by eplata31 5 years ago

Most upvoted comments

@devop911 comments like that aren’t super constructive or helpful. We absolutely want to solve this problem, but we also don’t want to regress performance in user’s applications for an edge case that is relatively uncommon. It’s a difficult situation to be in as a maintainer.

We’ll figure out a fix for this in time, it just needs a bit more work than a typical issue and we’ve been focused on shipping features for Octane. I agree, this is important though.

pzuraq on Aug 22, 2019

Fixed by https://github.com/emberjs/ember.js/pull/18296

rwjblue on Aug 23, 2019

I’m not sure we can fix this without regressing the common use case for get. If we can get a benchmark that shows the regression isn’t bad, then it’s probably fine to add this to get, but if not then we would need to build the fix directly into each-in. I suspect it would be a decent regression.

We could manually check for proxies and use unknownProperty on the object if it was a proxy, and otherwise use normal get syntax specifically when iterating an object using each-in, that may work as expected.

Long term, tracked props can’t come soon enough 😩

pzuraq on Aug 22, 2019

@devop911 my assessment of it being an edge case was based on the fact that we’ve seen very few reports of this bug overall since this issue was opened. If this were behavior that every Ember application relied on, it would have been a higher priority fix, absolutely, but it seems that not many folks use periods in their keys.

On the flip side, we do get lots of complaints whenever we regress performance. If this weren’t a hot path, the fix would definitely have come a lot earlier. Like I said, it’s a difficult situation to be in as a maintainer, especially when you’re trying to get things done 😩 I’ll see what we can do to get someone working on this sooner rather than later

@rwwagner90 I do think we can fix the behavior, so I’d rather not start warning, but if we can’t fix it I agree we should definitely warn users.

pzuraq on Aug 22, 2019

@amk221 Not the one you’re asking, but I ended up using this a bunch:

import { helper } from '@ember/component/helper';
import _ from 'lodash';

export function hashToArray([h]) {
  return _.map(h, function(value, key) {
    return {
      key: key,
      value: value,
    }
  });
}

export default helper(hashToArray);

iagox86 on Jul 18, 2019

@jasonmit could you send a PR with the failing test?

Definitely, I’ll do that now and link the issue.

ya, I only point it out because @jasonmit could possibly work around the issue by making the object implement Symbol.iterable

Great idea, I’ll try that out as a work around.

In case you’re curious why we have periods in the key, the keys in the object being iterated over are ISO strings (i.e., 2011-10-05T14:48:00.000Z) and those have periods before the UTC offset.

jasonmit on Jan 31, 2019

@chancancode - ya, I only point it out because @jasonmit could possibly work around the issue by making the object implement Symbol.iterable

rwjblue on Jan 30, 2019