fix(ssr-compiler): add SSR for `lwc:component` #4000

abdulsattar · 2024-02-19T11:30:52Z

Details

Adds support for SSRing lwc:component to the #3880 branch.

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

chore: add new keywords to package.json Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> chore: remove unnecessary non-null assertion Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> chore: improve internal type signature chore: address minor PR feedback chore: address more PR feedback chore: move serverSideRenderComponent to @lwc/ssr-runtime chore: pretty chore: remove acorn from @lwc/ssr-compiler chore: remove the test command from subpackages chore: move reserved keywords over to @lwc/shared chore: revert unnecessary rollup change chore: remove non-null assertions chore: address PR feedback chore: address PR feedback chore: address PR feedback chore: address PR feedback chore: clean up types in estemplate chore: minor cleanup chore: support backslash path separators chore: remove redundant NonNullable chore: minor cleanup fix: prop values of null or undefined should be rendered as empty text node chore: add a bunch of comments

abdulsattar · 2024-02-19T18:34:12Z

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

@@ -133,4 +133,13 @@ export function addGenerateMarkupExport(
    program.body.push(
        bGenerateMarkup(attrsAugmentation, classIdentifier, renderCall, classIdentifier)
    );
+    program.body.push(


We need this because dynamic components are imported in the JS file like so:

export class extends LightningElement { x() { ctor = await import('x/test'); } }

This is a default import and it'll import only the LightningElement. But for SSR, we want the generateMarkup function for that component as well which is exported as a named export and isn't present in the ctor.

I think I might be missing a piece of context for how we handle dynamic imports. Why is the resolved value of import('x/test') the default export from that component? Normally, the resolved value would be the module namespace object, which should include generateMarkup if that's a named export.

You're right, people actually do:

const {default: ctor} = await import('x/test');

and use the ctor like so:

<lwc:component is={ctor}></lwc:component>

So, the engine doesn't have access to the module namespace object, but just the default export.

divmain · 2024-02-20T01:25:00Z

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

@@ -133,4 +133,13 @@ export function addGenerateMarkupExport(
    program.body.push(
        bGenerateMarkup(attrsAugmentation, classIdentifier, renderCall, classIdentifier)
    );
+    program.body.push(


I think I might be missing a piece of context for how we handle dynamic imports. Why is the resolved value of import('x/test') the default export from that component? Normally, the resolved value would be the module namespace object, which should include generateMarkup if that's a named export.

divmain · 2024-02-20T01:29:01Z

packages/@lwc/ssr-compiler/src/index.ts

+    name: string;
+    namespace: string;
+}
+export interface TemplateTransformOptions {


Once/if ssr-compiler is the official path forward, we might want to define this interface as a subset of the compiler's TransformOptions, possibly by pulling it out of both into @lwc/shared or something like that.

For now, I think defining the interface here works well and should be left as is.

divmain · 2024-02-20T01:35:28Z

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

-    cxt.hoist(componentImport, childGeneratorLocalName);
-    const childTagName = node.name;
-
+export function ImportedComponent(


Could you add a comment here to disambiguate ImportedComponent from Component?

divmain · 2024-02-20T01:39:37Z

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

+
+export const Component: Transformer<IrComponent> = function Component(node, cxt) {
+    // Import the custom component's generateMarkup export.
+    const childGeneratorLocalName = `generateMarkup_${toPropertyName(node.name)}`;


I think node.name can have hyphens in it here. If so, that would be an invalid JS identifier, so we'll need to transform it into something compliant.

Changed it to use a uid.

And added a test for a component that has two child components.

divmain · 2024-02-20T01:43:24Z

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

+    return directive.name === 'Is';
+}
+export const LwcDynamic: Transformer<IrComponent> = function LwcDynamic(node, cxt) {
+    const dynamicDirective = node.directives.find(isLwcIsDirective);


If there is more than one lwc:is attribute, this will just return the first and ignore any that follow. Is this the same behavior as in engine-core?

Parser throws LWC1058: Invalid HTML syntax: duplicate-attribute. For more information in both cases.

nolanlawson · 2024-02-22T00:39:34Z

packages/@lwc/engine-server/src/__tests__/fixtures/component/modules/x/component/component.html

@@ -1,3 +1,4 @@
 <template>
    <x-child></x-child>
+    <x-anotherchild></x-anotherchild>


I don't see a test here for <lwc:component> ...?

BTW this is a good example of why it'd be nice to have the compiler output as part of the snapshots... would help with visualizing the generated code.

This tests the import statement we add when we see a child in the template. lwc:component are imported in the JS by the user.

Specifically, here db3322f#diff-17a5fc2e6a05c3f1e674d6942273c831cb914d8d3c3286062c429e5054465bdfR101, we increment UID to add unique import statements for each child.
e.g.

import { generateMarkup as generateMarkup_1 } from 'x/child'; import { generateMarkup as generateMarkup_2 } from 'x/anotherchild';

Definitely agree with having the compiler output in snapshots. This would have been clearer.

divmain and others added 10 commits February 9, 2024 16:36

chore: update fixtures to accommodate SSR compiler

1364d55

chore: add package-local test command

a5425a2

feat: experimental SSR compiler

3b256d6

chore: re-add acorn for estemplate

bf43fac

chore: test changes

923e065

chore: fixes to esTemplate, tests, & follow-up fixes

364c638

chore: remove no-op

c6da245

chore: update README

fb0645c

fix(ssr-compiler): add SSR for lwc:component

a5139bc

abdulsattar requested a review from a team as a code owner February 19, 2024 11:30

abdulsattar commented Feb 19, 2024

View reviewed changes

divmain requested changes Feb 20, 2024

View reviewed changes

abdulsattar added 3 commits February 20, 2024 19:13

fix(ssr-compiler): generate uid for every imported child generateMarkup

db3322f

fix(ssr-compiler): add test for two components

3d902e5

fix: add comments

b466dd3

nolanlawson reviewed Feb 22, 2024

View reviewed changes

divmain force-pushed the divmain/ssr-compiler branch 3 times, most recently from 5b2f725 to 3377347 Compare April 25, 2024 07:37

Base automatically changed from divmain/ssr-compiler to master April 25, 2024 07:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ssr-compiler): add SSR for `lwc:component` #4000

fix(ssr-compiler): add SSR for `lwc:component` #4000

abdulsattar commented Feb 19, 2024 •

edited

abdulsattar Feb 19, 2024 •

edited

divmain Feb 20, 2024

abdulsattar Feb 20, 2024 •

edited

divmain Feb 20, 2024

divmain Feb 20, 2024

divmain Feb 20, 2024

divmain Feb 20, 2024

abdulsattar Feb 20, 2024

abdulsattar Feb 20, 2024

divmain Feb 20, 2024

abdulsattar Feb 21, 2024

nolanlawson Feb 22, 2024

abdulsattar Feb 22, 2024

fix(ssr-compiler): add SSR for lwc:component #4000

Are you sure you want to change the base?

fix(ssr-compiler): add SSR for lwc:component #4000

Conversation

abdulsattar commented Feb 19, 2024 • edited

Details

Does this pull request introduce a breaking change?

Does this pull request introduce an observable change?

GUS work item

abdulsattar Feb 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdulsattar Feb 20, 2024 • edited

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

fix(ssr-compiler): add SSR for `lwc:component` #4000

fix(ssr-compiler): add SSR for `lwc:component` #4000

abdulsattar commented Feb 19, 2024 •

edited

abdulsattar Feb 19, 2024 •

edited

abdulsattar Feb 20, 2024 •

edited