feat: add dynamic components for @lwc/ssr-compiler #4847

jmsjtu · 2024-11-13T01:49:52Z

Details

This PR adds support for dynamic components in @lwc/ssr-compiler.

Does this pull request introduce a breaking change?

😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

🤞 No, it does not introduce an observable change.

GUS work item

W-16872192

…omponents

jmsjtu · 2024-11-13T02:13:40Z

packages/@lwc/ssr-compiler/src/compile-template/shared.ts

@@ -95,3 +103,38 @@ export function normalizeClassAttributeValue(value: string) {
    // @ts-expect-error weird indirection results in wrong overload being picked up
    return StringReplace.call(StringTrim.call(value), /\s+/g, ' ');
 }
+
+export function getChildAttrsOrProps(


I moved this function to the shared file because I'm reusing it in lwc-component.ts.

Open to keeping it in the Component.ts if it's got things specific to Components only.

jmsjtu · 2024-11-13T02:23:33Z

packages/@lwc/ssr-compiler/src/compile-template/transformers/lwc-component.ts

+        return [
+            bIfLwcIsExpressionDefined(
+                lwcIsExpression,
+                bifLwcIsExpressionTypeCorrect(
+                    lwcIsExpression,
+                    bYieldFromDynamicComponentConstructorGenerator(
+                        getChildAttrsOrProps(node.properties, cxt),
+                        getChildAttrsOrProps(node.attributes, cxt),
+                        lwcIsExpression
+                    ),
+                    bThrowErrorForInvalidConstructor(lwcIsExpression)
+                )
+            ),
+        ];


Here's a sample output:

if (instance.customCtor !== undefined && instance.customCtor !== null) { if (typeof instance.customCtor === "function" && instance.customCtor.prototype instanceof LightningElement) { const childProps = cloneAndDeepFreeze({}); const childAttrs = {}; yield* instance.customCtor[SYMBOL__GENERATE_MARKUP](null, childProps, childAttrs); } else { throw new Error(`Invalid constructor ${String(instance.customCtor)} is not a LightningElement constructor.`); } }

nit: It's not immediately apparent to me why a bunch little builders were used instead of a larger esTemplate. This seems to work fine though - just not quite as readable.

I wasn't sure what parts we wanted to keep in an esTemplate vs using a builder but this helps clear things up!

I'll redo this to use an esTemplate, I think it'll be easier to read as well!

jmsjtu · 2024-11-13T02:31:20Z

packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts

@@ -30,6 +31,7 @@ type RenderCallExpression = SimpleCallExpression & {

 const bGenerateMarkup = esTemplate`
    export async function* generateMarkup(tagName, props, attrs, slotted, parent, scopeToken) {
+        tagName = tagName ?? ${/*component tag name*/ is.literal}


Sample output will look like this:

async function* generateMarkup$1(tagName, props, attrs, slotted, parent, scopeToken) { tagName = tagName ?? "x-test";

We need this to get the tag name that's passed to the transformer, otherwise, the constructor doesn't have a tag name to use.

divmain · 2024-11-13T06:18:40Z

packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts

+    // This is needed to generate markup for dynamic components which are invoked through
+    // the generateMarkup function on the constructor.
+    // At the time of generation, the invoker does not have reference to its tag name to pass as an argument.
+    const defaultTagName = b.literal(`${namespace}-${name}`);


Just double checking: is the name value here kebab-cased or camel-cased?

Just checked. Yes it is camel-cased. So this needs to be transformed.

lwc/packages/@lwc/rollup-plugin/src/index.ts

Lines 324 to 328 in 29a641e

// Extract module name and namespace from file path.

// Specifier will only exist for modules with alias paths.

// Otherwise, use the file directory structure to resolve namespace and name.

const [namespace, name] =

specifier?.split('/') ?? path.dirname(filename).split(path.sep).slice(-2);

Added a helper function!

divmain · 2024-11-13T06:20:25Z

packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts

@@ -87,10 +89,17 @@ const bAssignGenerateMarkupToComponentClass = esTemplate`
 export function addGenerateMarkupExport(
    program: Program,
    state: ComponentMetaState,
+    options: TransformOptions,


Not sure what @nolanlawson thinks about this, but it might be worth scoping this down to { name: string, namespace: string }.

divmain · 2024-11-13T06:24:51Z

packages/@lwc/ssr-compiler/src/compile-template/transformers/lwc-component.ts

+        return [
+            bIfLwcIsExpressionDefined(
+                lwcIsExpression,
+                bifLwcIsExpressionTypeCorrect(
+                    lwcIsExpression,
+                    bYieldFromDynamicComponentConstructorGenerator(
+                        getChildAttrsOrProps(node.properties, cxt),
+                        getChildAttrsOrProps(node.attributes, cxt),
+                        lwcIsExpression
+                    ),
+                    bThrowErrorForInvalidConstructor(lwcIsExpression)
+                )
+            ),
+        ];


nit: It's not immediately apparent to me why a bunch little builders were used instead of a larger esTemplate. This seems to work fine though - just not quite as readable.

…omponents

nolanlawson

First pass of a review.

packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts

nolanlawson · 2024-11-13T22:57:59Z

packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts

+    // This is needed to generate markup for dynamic components which are invoked through
+    // the generateMarkup function on the constructor.
+    // At the time of generation, the invoker does not have reference to its tag name to pass as an argument.
+    const defaultTagName = b.literal(`${namespace}-${name}`);


Just checked. Yes it is camel-cased. So this needs to be transformed.

lwc/packages/@lwc/rollup-plugin/src/index.ts

Lines 324 to 328 in 29a641e

// Extract module name and namespace from file path.

// Specifier will only exist for modules with alias paths.

// Otherwise, use the file directory structure to resolve namespace and name.

const [namespace, name] =

specifier?.split('/') ?? path.dirname(filename).split(path.sep).slice(-2);

packages/@lwc/ssr-compiler/src/compile-template/transformers/lwc-component.ts

nolanlawson · 2024-11-13T23:08:41Z

This part doesn't seem to be rendering correctly:

  const hostHasScopedStylesheets = tmplFn.hasScopedStylesheets || hasScopedStaticStylesheets(["customCtor"]);
  // ...
  yield* tmplFn(props, attrs, slotted, ["customCtor"], instance);

Notice the ["customCtor"] which I think should be props.customCtor.

nolanlawson · 2024-11-13T23:10:08Z

I also notice that we don't have any tests for slots passed in to a dynamic component, or scope token (scoped style) classes on the dynamic component. We should add these for safety.

…omponents

…/dynamic-components

nolanlawson · 2024-11-15T00:20:20Z

Looks like slotted content is still not working; opened a PR with tests to confirm: #4872.

The good news though is that scoped slots are broken already so we don't have to handle those 😆 #4871

nolanlawson

We can get this PR in now and then figure out slotted content in a follow-up PR. Looks good!

jmsjtu · 2024-11-15T00:39:12Z

@nolanlawson sounds good, I'll work on the follow-up PR to get the slotted content in!

jmsjtu added 6 commits November 12, 2024 12:28

chore: first commit

5395e79

feat: add dynamic components for ssr-compiler

78eded2

test: update test fixtures

81ec6de

chore: remove config file

87ed874

chore: cleanup code

871fdf5

chore: add comments

67d62c7

jmsjtu requested a review from a team as a code owner November 13, 2024 01:49

jmsjtu added 3 commits November 12, 2024 18:01

Merge branch 'master' of github.com:salesforce/lwc into jtu/dynamic-c…

772bc55

…omponents

chore: resolve merge conflicts

e310b9b

chore: remove comment

37c8499

jmsjtu commented Nov 13, 2024

View reviewed changes

divmain reviewed Nov 13, 2024

View reviewed changes

jmsjtu added 2 commits November 13, 2024 11:50

chore: add dependencies

7712268

Merge branch 'master' of github.com:salesforce/lwc into jtu/dynamic-c…

ab92e9e

…omponents

nolanlawson reviewed Nov 13, 2024

View reviewed changes

packages/@lwc/ssr-compiler/src/compile-template/transformers/lwc-component.ts Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into jtu/dynamic-components

371318d

jmsjtu and others added 7 commits November 13, 2024 17:04

Merge branch 'master' of github.com:salesforce/lwc into jtu/dynamic-c…

edf8420

…omponents

chore: fix them tests

2746e71

refactor: use estemplate instead of builder and address pr feedback

9ba0901

test: fix test failures

2bb0491

chore: fix error message wording

6ca1545

Merge remote-tracking branch 'origin/master' into jtu/dynamic-components

ad1a403

Merge remote-tracking branch 'origin/jtu/dynamic-components' into jtu…

8cd5d6a

…/dynamic-components

nolanlawson mentioned this pull request Nov 15, 2024

test(ssr): add more tests for dynamic components #4872

Open

nolanlawson approved these changes Nov 15, 2024

View reviewed changes

jmsjtu merged commit b777da2 into master Nov 15, 2024
11 checks passed

jmsjtu deleted the jtu/dynamic-components branch November 15, 2024 00:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add dynamic components for @lwc/ssr-compiler #4847

feat: add dynamic components for @lwc/ssr-compiler #4847

jmsjtu commented Nov 13, 2024 •

edited

Loading

jmsjtu Nov 13, 2024 •

edited

Loading

jmsjtu Nov 13, 2024

divmain Nov 13, 2024

jmsjtu Nov 13, 2024

jmsjtu Nov 14, 2024

jmsjtu Nov 13, 2024

divmain Nov 13, 2024

nolanlawson Nov 13, 2024

jmsjtu Nov 14, 2024

divmain Nov 13, 2024

jmsjtu Nov 14, 2024

divmain Nov 13, 2024

nolanlawson left a comment

nolanlawson Nov 13, 2024

nolanlawson commented Nov 13, 2024

nolanlawson commented Nov 13, 2024

nolanlawson commented Nov 15, 2024

nolanlawson left a comment

jmsjtu commented Nov 15, 2024

	// Extract module name and namespace from file path.
	// Specifier will only exist for modules with alias paths.
	// Otherwise, use the file directory structure to resolve namespace and name.
	const [namespace, name] =
	specifier?.split('/') ?? path.dirname(filename).split(path.sep).slice(-2);

feat: add dynamic components for @lwc/ssr-compiler #4847

feat: add dynamic components for @lwc/ssr-compiler #4847

Conversation

jmsjtu commented Nov 13, 2024 • edited Loading

Details

Does this pull request introduce a breaking change?

Does this pull request introduce an observable change?

GUS work item

jmsjtu Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson commented Nov 13, 2024

nolanlawson commented Nov 13, 2024

nolanlawson commented Nov 15, 2024

nolanlawson left a comment

Choose a reason for hiding this comment

jmsjtu commented Nov 15, 2024

jmsjtu commented Nov 13, 2024 •

edited

Loading

jmsjtu Nov 13, 2024 •

edited

Loading