salesforce / lwc

⚡️ LWC - A Blazing Fast, Enterprise-Grade Web Components Foundation
https://lwc.dev
Other
1.64k stars 393 forks source link

Allow shorthand for directives like `lwc:ref` #3303

Open AllanOricil opened 1 year ago

AllanOricil commented 1 year ago

Is your feature request related to a problem? Please describe.

no, I just would like to know if it could be simplified. And nice feature, by the way. We can get rid of data-id parameters in our templates.

https://developer.salesforce.com/blogs/2023/01/lwc-enhancements-for-developers-learn-moar-spring-23 image

Is there a reason for prepending "ref" with the word "lwc:" ?

Couldn't it just be

<div :ref="reference"></div>
<div ref="reference"></div>

Vue and React template engines are able to use just "ref", so I believe lwc would also be able to do it. It would be good if you could try to follow what other frameworks did well, so that people that are used to one of them can learn lwc faster. I only see benefits doing so.

Another question, does it accept expressions?

Like,

<div ref={dynamicRef}></div>
<template for:each={bla} for:item="blas">
    <div ref={bla}></div>
</template>
<template for:each={bla} for:item="blas">
     <div ref={getDynamicRef(bla)}></div>
</template>
nolanlawson commented 1 year ago

Thanks for the feedback!

Is there a reason for prepending "ref" with the word "lwc:" ?

Yep, as mentioned in https://github.com/salesforce/lwc/issues/2503#issuecomment-1398793158, this is to make it clear what is native to the web platform and what is part of LWC. This tells developers whether they should be searching for documentation on MDN or on Salesforce/lwc.dev. 🙂

Another question, does it accept expressions?

No. lwc:ref must be a static string value. The for:each use case is something we specifically called out in the RFC as not yet implemented, but worth implementing. I opened an issue to track that: https://github.com/salesforce/lwc/issues/3304

Closing this issue, as we don't currently plan to shorten lwc:ref to ref. Thanks again for the feedback!

nolanlawson commented 1 year ago

Reopening this. After some internal discussion, it seems that it may be reasonable to support :ref in addition to lwc:ref. In fact, this is doable for all the lwc:* directives: :if, :render-mode, :preserve-comments, etc.

This might also be a good time to harmonize some of the directives that don't start with lwc:, e.g.:

The above two could be done in a backwards-compatible way; they would just be synonyms.

git2gus[bot] commented 1 year ago

This issue has been linked to a new work item: W-12415033

AllanOricil commented 1 year ago

@nolanlawson @jmsjtu

for:each becoming lwc:each feels not natural.

In my opinion, reading for:each={contacts} has always been weird because when I say/write the word each in english, the next word must be a singular one. For instance, I can't write or speak "for each bananas" in english. If I wanted to use plural, I would have to write it like "for each of these bananas". With that said, the argument is that you should remove the word each because it does not feel grammarly correct or natural when writing/speaking the directive using a plural word.

I would love if you could actually change the for:each directive entirely. Something like:

for:each={item in collection} for:each={(item, index) in collection}

I also believe that joining the two directives in one will simplify the template engine because you won't need to parse separately 2 directives and then merge their information before validating if it is correct or not.

It is similar to vue. The only difference is the addition of the word each, which I think it makes the directive sound better. I don't know, but I have the impression that I won't forget it if I stop using it. I won't need to look up the docs just to remember how to do a for each in lwc.

would be much better because it would reduce the amount of characters in a template, while also respecting the English grammar and keeping the word each. I could not find a RFC discussion about this improvement.

But if you can't really change it. What about using lwc:for and lwc:item? Then the correspondent shorthands would be: :for, :item and :key

caridy commented 1 year ago

I'm up for shorting lwc: to just :, but I will probably continue to use lwc: myself, so it is very clear that this is intended to be LWC specific... the namespacing of HTML attributes is very clear, we namespace to prevent collisions.

pmdartus commented 1 year ago

As always thanks for raising this @AllanOricil.

I am also up for finishing the migration of all the LWC directives to lwc:* and enabling a shorthand version with the : prefix.

Over the last 5 years, we have learned how LWC developers are using the existing directives. And as part of the harmonization process, I would like to make sure we also fix some of the design flaws in the for:each directive:

AllanOricil commented 1 year ago

As always thanks for raising this @AllanOricil.

I am also up for finishing the migration of all the LWC directives to lwc:* and enabling a shorthand version with the : prefix.

Over the last 5 years, we have learned how LWC developers are using the existing directives. And as part of the harmonization process, I would like to make sure we also fix some of the design flaws in the for:each directive:

  • Have the lwc:key directive applied to the same element as the lwc:each directive. Currently, the key is applied to the children elements, which makes very little sense and prevents some optimizations like the usage of fragments in the diffing algo.
  • Make the key directive optional. I have seen components having to create a unique id using Math.random() just to use it as a value for the key directive.
  • Exposing the current index of the iteration without having to use the iterator:* directive.

@pmdartus Another question, If I mutate an array that is used in a for:each does lwc recreate all the elements in the DOM, or does it patch and remove the ones that are no longer used? For example: a for loop rendered 1000 todo items, and then the todo Items array is mutaded

this.todoItems = this.todoItems.filter((t)=>t.include))

Does lwc throw away all 1000 elements from the DOM and then mount new ones, or does it patch some elements and unmount the ones that were deleted from the array?

Not sure how Vue does it, but their doc says it has a "smart heurist algorithm" that it is used to avoid unnecessary changes to the DOM when an array used in a for loop is mutated. So, if lwc does not currently have something like this, maybe this could be another good optimization you could implement.

pmdartus commented 1 year ago

Another question, If I mutate an array that is used in a for:each does lwc recreate all the elements in the DOM, or does it patch and remove the ones that are no longer used?

Yes, it's something that is baked into LWC rendering engine from day 1. This is the reason why template iterations require a stable key directive. This key enables the engine to map items between 2 rendering cycles to apply the minimal set of DOM operations.

You can find this logic in here in LWC, and here in Vue.