salesforce / lwc

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

Add support for dynamic `<slot>` names #4207

Open nolanlawson opened 6 months ago

nolanlawson commented 6 months ago

Developers would like to be able to have dynamic <slot> names:

<!-- x-slottable -->
<template>
  <slot name={dynamic}></slot>
</template>
<!-- x-slotter -->
<template>
  <x-slottable>
    <div slot={dynamic}></slot>
  </x-slottable>
</template>

This is currently disallowed by our compiler (Error: LWC1076: Name attribute on slot tag can't be an expression.). But there's nothing in native shadow DOM that inherently disallows this; we could trivially require it there.

In fact, you can already achieve this in native shadow DOM by doing manual DOM manipulation, or cheating by using lwc:spread.

One question is whether we'd want to support this in synthetic shadow DOM (probably not) or light DOM (maybe).

cardoso commented 5 months ago

I tried implementing this and to my surprise it seems to work by simply removing the check:

if (ast.isExpression(nameAttribute.value)) {
    ctx.throwOnNode(ParserDiagnostics.NAME_ON_SLOT_CANNOT_BE_EXPRESSION, nameAttribute);
}

Am I missing something as I suspect? Should I submit a PR?

nolanlawson commented 5 months ago

Does it work if you run yarn && yarn test && yarn test:karma && DISABLE_SYNTHETIC=1 yarn test:karma? Curious whether it actually works across both synthetic and native shadow.

cardoso commented 5 months ago

@nolanlawson yes, it only fails the test for the NAME_ON_SLOT_CANNOT_BE_EXPRESSION error, which I believe would be updated / removed.

packages/@lwc/template-compiler/src/tests/fixtures/slots/error-definition-dynamic-name/expected.js

- Expected
+ Received

+ import { registerTemplate } from "lwc";
+ const stc0 = [];
+ function tmpl($api, $cmp, $slotset, $ctx) {
+   const { s: api_slot } = $api;
+   return [
+     api_slot(
+       "",
+       {
+         attrs: {
+           name: $cmp.slotName,
+         },
+         key: 0,
+       },
+       stc0,
+       $slotset
+     ),
+   ];
+   /*LWC compiler vX.X.X*/
+ }
+ export default registerTemplate(tmpl);
+ tmpl.slots = [""];
+ tmpl.stylesheets = [];

I'm guessing we would need a test for the actual feature and perhaps that would fail for non-native dom.

nolanlawson commented 4 months ago

I looked into this a bit further. What makes this complex is that we have a compile-time check to ensure you don't have duplicate slots in the same template:

<slot name="foo"></slot>
<slot name="foo"></slot> <!-- Duplicate -->

One reason to get rid of this check is that native shadow will happily render duplicate slots. It just ignores the second one.

We'd need to check first how this works with light/synthetic shadow though. And possibly mimic the native shadow behavior (or perhaps throw an error at runtime). This may or may not be backwards compatible, though (due to the lwc:spread workaround).