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

Glimmer Pre-flight Checklist #13644

Closed
chadhietala opened this issue Jun 10, 2016 · 6 comments
Closed

Glimmer Pre-flight Checklist #13644

chadhietala opened this issue Jun 10, 2016 · 6 comments
Labels

Comments

@chadhietala
Copy link
Contributor

chadhietala commented Jun 10, 2016

You're closer than you think

Below is the pre-flight checklist to launch Glimmer. If you are going to take a test to fix please leave a comment on this thread. Please mention this issue in your PR so we can track more effectively.

Features

Testing

Failing Tests

{{render}} test failures

Cruff that can be removed?

Real world applications

  • Ensure ember-twiddle works
  • Simple apps work
  • Ember inspector
  • Get a production app running
  • Deploy an app to production to do a sanity check
  • Remove the glimmer flag on master

Unspecified Behavior

The following are tests that are failing and will back port if we find out this is a widely used use case, however the semantics here are just wrong.

@rwjblue
Copy link
Member

rwjblue commented Jun 10, 2016

Thanks for putting this together @chadhietala

@mmun
Copy link
Member

mmun commented Jun 17, 2016

I'll take a stab at {{render}}.

@rwjblue
Copy link
Member

rwjblue commented Jun 17, 2016

@mmun - I updated the description to mark it as 🔒 'ed.

fivetanley pushed a commit to fivetanley/ember.js that referenced this issue Jun 19, 2016
Before, the test rendered the following:

```handlebars
<div data-foo="{{if cond1 \"T1\" \"F1\"}}"></div>
<div data-foo="{{if cond2 \"T2\" \"F2\"}}"></div>
```

The test was only asserting against the value of the `data-foo` of the
first div, causing it fo fail because it was missing the value of the
`data-foo` in the second div.

This is because the test was using `wrappedTemplateFor`, which
is a convenience method which only expects *one* template to be
rendered at a time.

Instead, we use `wrapperFor` and two `templateFor` calls to instead
create only *one* div and assert against it. What's rendered in the test
now looks like:

```handlebars
<div data-foo="{{if cond1 \"T1\" \"F1\"}}{{if cond2 \"T2\" \"F2\"}}"></div>
```

refs emberjs#13644
fivetanley pushed a commit to fivetanley/ember.js that referenced this issue Jun 19, 2016
In Ember, you can remove duplicates in an array by providing the `key`
option to the `{{#each}}` helper.

For example, if given the following `model` property:

```javascript
[
  {
    "id": "1",
    "name": "Robert Jackson"
  },
  {
    "id": "1",
    "name": "Robert Jackson with diff name but same ID"
  },
  {
    "id": "2",
    "name": "Katie Gengler"
  }
]
```

And the following template:

```handlebars
{{#each model key="id" as |person|
  {{person.name}}
{{/each}}
```

It would render the following:

```html
Robert Jackson
Katie Gengler
```

Even though we have two objects with "Robert Jackson" as the name, they
share the same "id", so Ember will only render the first object it
encounters with that "id" while iterating. That's why "Robert Jackson"
shows up but not "Robert Jackson with diff name but same ID".

This test was adding in a new object (in our case `{text: ' '}`), but an
object with `{text: ' '}` already existed in the setup line. Because it
was cached by the same `'text'` value, it did not show up in the
subsequent rerender.

We change it to a different text value so we can assert it was added
correctly in the list.

refs emberjs#13644
@zackthehuman
Copy link
Contributor

I'm working on a change in Glimmer to fix the following:

  • should property escape unsafe hrefs
  • blacklists href bindings based on protocol

See glimmerjs/glimmer-vm#197 for more info.

@zackthehuman
Copy link
Contributor

I'm working on fixing array iteration. PR here: #13763

Related tests:

  • {{each}} it can render duplicate primitive items
  • {{each}} it can render duplicate objects

@rwjblue
Copy link
Member

rwjblue commented Sep 8, 2016

I believe this is taken care of, closing...

@rwjblue rwjblue closed this as completed Sep 8, 2016
toddjordan pushed a commit to toddjordan/ember.js that referenced this issue Sep 9, 2016
Before, the test rendered the following:

```handlebars
<div data-foo="{{if cond1 \"T1\" \"F1\"}}"></div>
<div data-foo="{{if cond2 \"T2\" \"F2\"}}"></div>
```

The test was only asserting against the value of the `data-foo` of the
first div, causing it fo fail because it was missing the value of the
`data-foo` in the second div.

This is because the test was using `wrappedTemplateFor`, which
is a convenience method which only expects *one* template to be
rendered at a time.

Instead, we use `wrapperFor` and two `templateFor` calls to instead
create only *one* div and assert against it. What's rendered in the test
now looks like:

```handlebars
<div data-foo="{{if cond1 \"T1\" \"F1\"}}{{if cond2 \"T2\" \"F2\"}}"></div>
```

refs emberjs#13644
toddjordan pushed a commit to toddjordan/ember.js that referenced this issue Sep 9, 2016
In Ember, you can remove duplicates in an array by providing the `key`
option to the `{{#each}}` helper.

For example, if given the following `model` property:

```javascript
[
  {
    "id": "1",
    "name": "Robert Jackson"
  },
  {
    "id": "1",
    "name": "Robert Jackson with diff name but same ID"
  },
  {
    "id": "2",
    "name": "Katie Gengler"
  }
]
```

And the following template:

```handlebars
{{#each model key="id" as |person|
  {{person.name}}
{{/each}}
```

It would render the following:

```html
Robert Jackson
Katie Gengler
```

Even though we have two objects with "Robert Jackson" as the name, they
share the same "id", so Ember will only render the first object it
encounters with that "id" while iterating. That's why "Robert Jackson"
shows up but not "Robert Jackson with diff name but same ID".

This test was adding in a new object (in our case `{text: ' '}`), but an
object with `{text: ' '}` already existed in the setup line. Because it
was cached by the same `'text'` value, it did not show up in the
subsequent rerender.

We change it to a different text value so we can assert it was added
correctly in the list.

refs emberjs#13644
toddjordan pushed a commit to toddjordan/ember.js that referenced this issue Sep 9, 2016
toddjordan pushed a commit to toddjordan/ember.js that referenced this issue Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants