-
Notifications
You must be signed in to change notification settings - Fork 25
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
RFC: List rendering reform #84
base: master
Are you sure you want to change the base?
Changes from 3 commits
29ec4b0
08df57b
c3bd69a
2195f72
4dbf178
8b594e4
fd0471b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,267 @@ | ||
--- | ||
title: List rendering reform | ||
status: DRAFTED | ||
created_at: 2023-11-14 | ||
updated_at: 2023-11-14 | ||
pr: https://github.com/salesforce/lwc-rfcs/pull/84 | ||
--- | ||
|
||
# List rendering reform | ||
|
||
## Summary | ||
|
||
This RFC proposes a new template directive, `lwc:each`, which resolves issues and provides enhancements compared to the existing `for:each` and `iterator:*` directives. | ||
|
||
## Basic example | ||
|
||
```html | ||
<template> | ||
<ul> | ||
<template lwc:each={contacts} | ||
lwc:item="contact" | ||
lwc:key={contact.id} | ||
lwc:index="index" | ||
lwc:first="first" | ||
lwc:last="last" | ||
lwc:even="even" | ||
lwc:odd="odd"> | ||
<li> | ||
Name: {contact.name} | ||
Index: {index} | ||
First? {first} | ||
Last? {last} | ||
Even? {even} | ||
Odd? {odd} | ||
</li> | ||
</template> | ||
</ul> | ||
</template> | ||
``` | ||
|
||
## Motivation | ||
|
||
This proposal supersedes the existing [`for:each`](https://lwc.dev/guide/html_templates#for%3Aeach) and [`iterator:*`](https://lwc.dev/guide/html_templates#iterator) directives and resolves several issues with them: | ||
|
||
- We have two directives for lists instead of one, with different features available on each. The new directive unifies them. | ||
- The existing directives do not use the `lwc:` prefix, unlike every other modern LWC directive. Using `lwc:` makes it clear which attributes are LWC-specific, and would allow for improvements in the future such as [shorthands](https://github.com/salesforce/lwc/issues/3303) (out of scope for this RFC). | ||
- The key should be defined on the list root element, not on each element inside the list. The current behavior requires tediously repeating the `key` for each element inside the list, and also leads to [inconsistent behavior](https://github.com/salesforce/lwc/issues/3860) for text/comment nodes. | ||
- There is no way to easily render based on even vs odd list items. | ||
|
||
### Prior art | ||
|
||
- [Vue `v-for`](https://vuejs.org/guide/essentials/list.html) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing from this proposal: Vue has a way of iterating over objects (key/value) as well as array-likes. Given that |
||
- [Svelte `{#each}`](https://svelte.dev/docs/logic-blocks#each) | ||
- [Angular `*ngFor`](https://angular.io/api/common/NgForOf#local-variables) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: Angular has |
||
- [Solid `<For>`](https://docs.solidjs.com/references/api-reference/control-flow/For) | ||
- [Lit `repeat`](https://lit.dev/docs/templates/lists/#the-repeat-directive) | ||
|
||
## Detailed design | ||
|
||
The design of `lwc:each` is almost identical to that of `for:each`. There are only a few modifications. | ||
|
||
First, all directives are prefixed with `lwc:`: | ||
|
||
- `for:each` → `lwc:each` | ||
- `for:item` → `lwc:item` | ||
- `key` → `lwc:key` | ||
- `for:index` → `lwc:index` | ||
|
||
Second, the `lwc:key` must be declared on the iterator root, not on each item within the loop: | ||
|
||
```html | ||
<template lwc:each={items} lwc:item="item" lwc:key={item.id}> | ||
{item.name} | ||
</template> | ||
``` | ||
|
||
Third, four new convenience directives are added: | ||
|
||
- `lwc:first` - boolean that is `true` if the item is first in the list | ||
- `lwc:last` - boolean that is `true` if the item is last in the list | ||
- `lwc:even` - boolean that is `true` if the item index is even | ||
- `lwc:odd` - boolean that is `true` if the item index is odd | ||
|
||
Each directive will be detailed separately. | ||
|
||
### `lwc:each` | ||
|
||
This functions nearly identically to `for:each`. (That's kind of the point – it should not be difficult for component authors to migrate.) Like `for:each`, it is supported in both the `<template>` format: | ||
|
||
```html | ||
<template lwc:each={items} lwc:item="item" lwc:key={item.id}> | ||
{item.name} | ||
</template> | ||
``` | ||
|
||
…as well as the shorthand, `<template>`-less format: | ||
|
||
```html | ||
<li lwc:each={items} lwc:item="item" lwc:key={item.id}> | ||
{item.name} | ||
</li> | ||
``` | ||
|
||
Identically to `for:each`, it supports any [Iterable](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols). | ||
|
||
### `lwc:item` | ||
|
||
Identical to `for:item`. It is required for `lwc:each`, and throws a compile-time error if it is not defined: | ||
|
||
lwc:each and lwc:item directives should be associated together. | ||
|
||
(This is the same error thrown for `for:each` in the same situation.) | ||
|
||
[Unlike `for:item`](https://stackblitz.com/edit/salesforce-lwc-m56edb?file=src%2Fmodules%2Fx%2Fapp%2Fapp.html,src%2Fmodules%2Fx%2Fapp%2Fapp.js&title=LWC%20playground), `lwc:item` cannot use the same variable name as `lwc:each`: | ||
|
||
```html | ||
<template lwc:each={foo} lwc:item="foo" lwc:key={foo.id}> | ||
</template> | ||
``` | ||
|
||
The above will throw a compile-time error, since otherwise the behavior may be unexpected and confusing, especially when considering the `lwc:key` on the same element. (Which `foo` does `lwc:key={foo.id}` refer to above? Rather than guessing, we will throw a compile-time error.) | ||
|
||
### `lwc:key` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One usecase that is not very clear about existing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly. With There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we might need to slightly distinguish the way <li for:each={items} for:item="item" key={item.id}>
{item.name}
</li> In the example, the key is applied to the The reason I mention this is because when there is a nested loop where the outer loop is a stackblitz example (playground link) <template>
<template for:each={contacts} for:item="contact">
<div for:each={contact.addresses} for:item="address" key={contact.id}>
<section>{address.zip}</section>
</div>
</template> Note the If I understood correctly, with <template>
<template lwc:each={contacts} lwc:item="contact" lwc:key={contact.id}>
<div lwc:each={contact.addresses} lwc:item="address" lwc:key={address.id}>
<section>{address.zip}</section>
</div>
</template> How would the As long as there's a key on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jmsjtu I think the example you provide is a bug in In the case you mention, I think the problem arises from the ambiguity of what, exactly, the <template for:each={contacts} for:item="contact">
<div key={contact.id} for:each={contact.addresses} for:item="address"></div>
</template> In the above example, does the Another benefit of this RFC, IMO, is that |
||
|
||
Identical to `key`, except that it must be declared on the same element as `lwc:each`. This avoids the tedious repetition required for `key`: | ||
|
||
```html | ||
<template for:each={items} for:item="item"> | ||
<div key={item.id}></div> | ||
<span key={item.id}></span> | ||
<button key={item.id}></button> | ||
</template> | ||
``` | ||
|
||
With the new directives, this becomes: | ||
|
||
```html | ||
<template lwc:each={items} lwc:item="item" lwc:key={item.id}> | ||
<div></div> | ||
<span></span> | ||
<button></button> | ||
</template> | ||
``` | ||
|
||
Also note that, in the shorthand `<template>`-less format, `lwc:ref` is defined not on a `<template>` but on the same element as the `lwc:each`: | ||
nolanlawson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```html | ||
<li lwc:each={items} lwc:item="item" lwc:key={item.id}> | ||
{item.name} | ||
</li> | ||
``` | ||
|
||
Similar to `key`, `lwc:key` is required on elements with `lwc:each`, and throws an error if missing: | ||
|
||
Missing lwc:key for lwc:each. Iterators must have a unique, computed key value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to discuss whether or not we keep this constraint with the new syntax. IMO, we should make it optional but strongly encouraged. The main reason we ask developers to set a key on iteration is to improve list diffing and re-ordering. However, components don't always render a list of keyed items. It is currently awkward for developers to render a list of text in LWC. export default class extends LightningElement {
tags = ["performance", "bug fix", "trust"]
} To wrap around this limitation. developers usually use the text itself as a key. <template for:each={tags} for:item="tag">
<div class="tag-wrapper" key={tag}>{tag}</div>
</template> However, this approach is problematic as it only works if the list contains unique values. To solve this issue, I have seen some LWC components generating random ids for each entry which is also not ideal. Now that LWC developers are used to key their iterations, I don't think that it would be problematic to relax this restriction. As a side note, I am not aware of any other UI framework enforcing this restriction. Update: I see that it is already discussed in "Making There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also discussed in #84 (comment). |
||
|
||
Unlike `key`, a missing `lwc:key` throws the error regardless of the content inside the iterator. (A missing `key` will not throw for text nodes, comment nodes, or empty markup – an apparent [inconsistency](https://github.com/salesforce/lwc/issues/3860) in that directive.) | ||
|
||
Similar to `key`, `lwc:key` must not reference the variable defined by `lwc:index`. It also must not reference the variable defined by `lwc:first`, `lwc:last`, `lwc:even`, or `lwc:odd`. For example, this will throw a compile-time error: | ||
|
||
```html | ||
<template lwc:each={items} lwc:item="item" lwc:index="idx" lwc:key={idx}> | ||
</template> | ||
``` | ||
|
||
In short, `lwc:key` _must_ reference the `lwc:item` variable. Anything else throws a compile-time error. | ||
|
||
### `lwc:index` | ||
|
||
Identical to `for:index`. Returns a number for the current list index. | ||
|
||
As with `for:index`, `lwc:index` cannot use the same string as the `lwc:item` on the same element: | ||
|
||
```html | ||
<template lwc:each={items} lwc:key={same.id} lwc:item="same" lwc:index="same"> | ||
</template> | ||
``` | ||
|
||
The above will throw a compile-time error. | ||
|
||
(Note that `for:index` only [incidentally](https://stackblitz.com/edit/salesforce-lwc-se51rr?file=src%2Fmodules%2Fx%2Fapp%2Fapp.html,src%2Fmodules%2Fx%2Fapp%2Fapp.js&title=LWC%20playground) throws an error for the same case, because the `key` ends up referencing the `for:index` variable. For `lwc:index`, the compile-time error should be explicit.) | ||
|
||
### `lwc:first`, `lwc:last`, `lwc:even`, `lwc:odd` | ||
|
||
These new directives take strings as their values. When referenced, they resolve to a boolean representing whether the list item is first, last, even, or odd. (This is heavily inspired by [Angular](https://angular.io/api/common/NgForOf#local-variables).) | ||
|
||
The addition of `lwc:first` and `lwc:last` serve to unify the two existing directives – previously, `iterator:*` was the only way to get "first" or "last" behavior. The addition of `lwc:even` and `lwc:odd` are merely for convenience (especially in the absence of [complex template expressions](https://github.com/salesforce/lwc/pull/3376)). | ||
|
||
nolanlawson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
### Constraints | ||
|
||
All new directives that take a string as their value (`lwc:item`, `lwc:index`, `lwc:first`, `lwc:last`, `lwc:even`, and `lwc:odd`) must have unique values when used on the same element. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any thoughts on using In LWC template syntax, the usage of quotes on an HTML attribute denotes a static binding. But in the case of For context, I was the one who originally pushed to wrap the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think using <c-child>
<template lwc:slot-data="item">
<span>{item.name}</span>
</template>
</c-child> In my mind, the quotes mean "declare a variable with a given name," not "evaluate a JS expression." Also, if we change |
||
|
||
For example, the following will throw a compile-time error, because `lwc:item` and `lwc:index` have the same values: | ||
|
||
```html | ||
<template lwc:each={items} lwc:key={same.id} lwc:item="same" lwc:index="same"> | ||
</template> | ||
``` | ||
|
||
Empty values (e.g. `lwc:index=""`) will also throw a compile-time error. For example: | ||
|
||
```html | ||
<template lwc:each={items} lwc:key={item.id} lwc:item="item" lwc:index=""> | ||
</template> | ||
``` | ||
|
||
Missing values (e.g. `lwc:index`) will also throw at compile-time: | ||
|
||
```html | ||
<template lwc:each={items} lwc:key={item.id} lwc:item="item" lwc:index> | ||
</template> | ||
``` | ||
|
||
The directives may appear in any order on the element. | ||
|
||
Any other constraints that apply to `for:each` must also apply to `lwc:each`. For example, `lwc:ref` is (currently) unsupported inside of `for:each` and `iterator:*` – the same applies to `lwc:each`. For another example, `<slot>`s cannot have any of the new directives, and cannot be repeated inside the new iterator. | ||
|
||
## Drawbacks | ||
|
||
This new directive may cause confusion, because we are introducing a third directive to try to unify the previous two (see [XKCD 927](https://xkcd.com/927/)). | ||
|
||
The implementation also becomes more complex, as we will need to support all three directives for the foreseeable future. Perhaps they can share logic under the hood, but we will still need to test all three, at the very least. | ||
nolanlawson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
We will also need to make an effort to advocate for adoption of the new directive, which may involve updating existing documentation and writing codemods or IDE tooling. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any thoughts about adding compiler warnings? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think similar to |
||
|
||
## Alternatives | ||
|
||
### Enhancing `for:each` | ||
|
||
One reasonable alternative would be to enhance the existing `for:each` rather than creating a new directive. However, this has several drawbacks: | ||
|
||
1. There could be conflicts between the legacy `key` on within-iterator elements versus the `lwc:key` on the iterator element. In cases of nested iterators, it could be very tricky to determine which keys should apply to which iterators. | ||
2. It does not move us closer to a world where all LWC directives use the `lwc:` prefix. Directives like `for:each`, `for:item`, `for:index`, and `key` would continue to be the way to write iterators, making it difficult to support a [directive shorthand](https://github.com/salesforce/lwc/issues/3303) in the future. | ||
|
||
### Enhancing `iterator:*` | ||
|
||
Similarly, enhancing `iterator:*` is another approach. However, this directive is seldom-used, and is rigid in how it allows for referencing the value, index, and first and last items in a list (the property names must always be `value`, `index`, `first`, and `last`). | ||
|
||
### Making `lwc:key` optional | ||
|
||
The key is a performance optimization, and not all developers need it. Some developers even use a getter with `Math.random()` to provide a random key, to work around the current LWC requirement that lists must have a `key`. | ||
|
||
It is entirely possible to make `lwc:key` optional. However, this comes with a few downsides: | ||
|
||
1. Developers may be encouraged to use a less-performant rendering pattern, since it takes less effort to omit the key. | ||
2. It deviates from the existing behavior of `for:each`, and is one more thing to teach. | ||
3. Developers may now assume that keys are actually a _bad_ idea, since the "improved" `lwc:each` makes them optional. | ||
|
||
So, for this proposal, keys are still required. However, it is definitely something that can be revisited in the future. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pmdartus: Use case for making it optional:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ekashida: We could add another directive like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into how other frameworks do this:
|
||
|
||
## Adoption strategy | ||
|
||
Similar to `lwc:if`/`lwc:elseif`/`lwc:else`, the messaging will be that `lwc:each` is the "new" way to use lists in LWC templates, and component authors should migrate from `for:each` and `iterator:*`. | ||
|
||
Unlike `lwc:if` and friends, there are no feature gaps between `lwc:each` and the existing `for:each` and `iterator:*` directives. (`lwc:if` is missing an equivalent to `if:false`, at least in the absence of complex expressions.) Therefore, it should be relatively straightforward to migrate existing components – it could even be done with [lwc-codemod](https://github.com/salesforce/lwc-codemod). | ||
|
||
In the future, we could also use [component-level API versioning](https://lwc.dev/guide/versioning#component-level-api-versioning) to disallow the legacy `for:each` and `iterator:*` directives. | ||
|
||
# How we teach this | ||
|
||
The new `lwc:each` directive is essentially a superset of `for:each`, so it can leverage most of the teaching material for that directive. | ||
|
||
In addition, we can downplay or remove documentaiton for `for:each` and `iterator:*`, simplifying the LWC education narrative. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also mark There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it might be a bit early to do this. Similar to |
||
|
||
# Unresolved questions | ||
|
||
None at this time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pmdartus raised another benefit: we could use Fragments as the root of each list item. This should make the vdom diffing much more efficient because we would be diffing fewer vnodes in cases where the list repeater has multiple top-level elements.
Today we can't do this due to
key
. A developer could technically put a differentkey
on every top-level element inside of a list, which would defeat the fragment optimization since the fragment must have one key. The new proposal solves this sincelwc:key
is hoisted.