Skip to content
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

feat: new compiler targeting SSR #3880

Merged
merged 4 commits into from
Apr 25, 2024
Merged

feat: new compiler targeting SSR #3880

merged 4 commits into from
Apr 25, 2024

Conversation

divmain
Copy link
Contributor

@divmain divmain commented Nov 23, 2023

This introduces a new compiler to the @lwc namespace. Today, there is a single compiler that generates per-component artifacts; these artifacts are invoked whether you're running against @lwc/engine-dom or @lwc/engine-server. This fact brings with it many constraints, two of which are particularly noteworthy when considering SSR use cases:

  • rendering on the server is synchronous
    • there is no mechanism to pause rendering
    • there is no mechanism to render a component twice/thrice/etc
    • there is no out-of-the-box way to await fetching of data in the midst of rendering
  • rendering on the server is slow;
    • the abstractions used in the server setting are roughly 1:1 to those used in the browser
    • many of these abstractions are extraneous and costly
    • these abstractions are only present because of the architectural choice of 1:1 browser/server implementation parity

The introduction of this new compiler would mean that LWCs are compiled twice:

  1. A JavaScript artifact is generated for use in the browser. The code in this artifact relies on @lwc/engine-dom, utilizes VDOM for diffing and rendering, and interacts with browser elements & other browser constructs.
  2. Another JavaScript artifact is generated for use in Node.js. The code in this second artifact relies on a very slim runtime, utilizes no intermediate representations, does not simulate or interact with elements, embraces the restrictions of an SSR environment, and is optimized for speed & asynchronous evaluation. The basic primitive of these new compilation artifacts is () => AsyncGenerator<string, never, void>.

Currently, the new compiler is wired up to the snapshot tests of @lwc/engine-server. It is only passing roughly 40% of the tests at present, and notably does not yet support slotted content, context, and various sundry. However, the code has taken shape over the course of the spike, such that it is becoming increasingly evident how those gaps may be closed.

An initial perf benchmark can be found here

I'm aiming to merge this PR such that we can iterate towards 100% compliance with the @lwc/engine-server test suite in smaller, incremental PRs. While it is possible to run the tests locally, the @lwc/ssr-compiler tests do not run during yarn run test and do not cause CI to fail. Once we're at 100%, we can turn on coverage officially.

@divmain
Copy link
Contributor Author

divmain commented Nov 23, 2023

Here's a comparison of the two compilation outputs of a simple component:

Today's compiler output:

'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

var lwc = require('lwc');

const $fragment1 = lwc.parseFragment`<div class="foo bar foo-bar${0}" data-foo="foo" style="color: red; background-color: blue;"${2}></div>`;
function tmpl($api, $cmp, $slotset, $ctx) {
  const {st: api_static_fragment} = $api;
  return [api_static_fragment($fragment1(), 1)];
  /*LWC compiler v4.0.1*/
}
var _tmpl = lwc.registerTemplate(tmpl);
tmpl.stylesheets = [];
tmpl.stylesheetToken = "lwc-4ol5nhdt6n2";
tmpl.legacyStylesheetToken = "x-attribute-static_attribute-static";
lwc.freezeTemplate(tmpl);

class AttributeStatic extends lwc.LightningElement {
  /*LWC compiler v4.0.1*/
}
var attributeStatic = lwc.registerComponent(AttributeStatic, {
  tmpl: _tmpl,
  sel: "x-attribute-static",
  apiVersion: 60
});

const tagName = 'x-attribute-static';

exports.default = attributeStatic;
exports.tagName = tagName;

SSR compiler output:

'use strict';

var runtime = require('@lwc/ssr-compiler/runtime');
require('@lwc/shared');

async function* tmpl(props, attrs, slotted, Cmp, instance) {
  if (Cmp.renderMode !== 'light') {
    yield '<template shadowroot="open">';
  }
  yield "<div class=\"foo bar foo-bar\" data-foo=\"foo\" style=\"color: red; background-color: blue;\"></div>";
  if (Cmp.renderMode !== 'light') {
    yield '</template>';
  }
}

class AttributeStatic extends runtime.LightningElement {}
async function* generateMarkup(tagName, props, attrs, slotted) {
  const instance = new AttributeStatic({
    tagName: tagName.toUpperCase()
  });
  Object.assign(instance, props);
  yield `<${tagName}`;
  yield* runtime.renderAttrs(attrs);
  yield '>';
  const tmplFn = tmpl ?? runtime.fallbackTmpl;
  yield* tmplFn(props, attrs, slotted, AttributeStatic, instance);
  yield `</${tagName}>`;
}

const tagName = 'x-attribute-static';

exports.generateMarkup = generateMarkup;
exports.tagName = tagName;

@divmain divmain force-pushed the divmain/ssr-compiler branch 3 times, most recently from 798b19d to e3de526 Compare February 6, 2024 09:03
@divmain divmain marked this pull request as ready for review February 6, 2024 09:09
@divmain divmain requested a review from a team as a code owner February 6, 2024 09:09
@divmain divmain changed the title [NO MERGE] feat: new compiler targeting SSR feat: new compiler targeting SSR Feb 6, 2024
Copy link
Contributor

@wjhsf wjhsf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly small things, because evaluating the big picture is hard when you make the picture too big to fit on the screen without an hour of scrolling. 😜

(part 1)

packages/@lwc/ssr-compiler/package.json Outdated Show resolved Hide resolved
packages/@lwc/ssr-runtime/package.json Outdated Show resolved Hide resolved
scripts/tasks/check-and-rewrite-package-json.js Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-template/shared.ts Outdated Show resolved Hide resolved
const ast = parseModule(src, {
module: true,
next: true,
}) as EsProgram;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame meriyah doesn't use @types/estree but exposes it's own types which are slightly different. These subtle differences could end up causing bigger issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on this issue it sounds like there's openness to refactoring on top of estree types; it's just a lot of work. The thing is, acorn also doesn't use estree types and instead reimplements them. So it seems like this is just a thing with JS parsers.

I may take a peak at the meriyah source to see how hard it'd be to contribute this change. And long term, despite the perf wins with meriyah, we can always use acorn like we're doing elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also a bit concerned about having so many JS parsers in our packages in general. We already have Babel and Acorn and estree-walker, now we would be adding meriyah. Just an opportunity for any one of those things to throw breaking changes at us over time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember why I brought in meriyah - acorn doesn't support Js features until they are stage 4. This includes decorators. So it was a choice between babel+acorn or meriyah. I hate the double-transform, especially when compilation might be in the hot path, which is why I went with meriyah. I'm open to alternatives.

Copy link
Contributor

@abdulsattar abdulsattar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits. LGTM, anything else can be addressed in later PRs.

packages/@lwc/ssr-compiler/src/generator.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-js/index.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-runtime/src/index.ts Show resolved Hide resolved
packages/@lwc/ssr-runtime/src/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a lot more time to grok this PR (this is one of the more theory-dense PRs we've had in a while!) but this is my initial feedback.

const ast = parseModule(src, {
module: true,
next: true,
}) as EsProgram;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also a bit concerned about having so many JS parsers in our packages in general. We already have Babel and Acorn and estree-walker, now we would be adding meriyah. Just an opportunity for any one of those things to throw breaking changes at us over time.

packages/@lwc/ssr-compiler/src/compile-template/shared.ts Outdated Show resolved Hide resolved
scripts/tasks/check-and-rewrite-package-json.js Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-js/catalog-tmpls.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/estemplate.ts Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/estemplate.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/estemplate.ts Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/estemplate.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-template/context.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-template/element.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-template/element.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-template/index.ts Outdated Show resolved Hide resolved
@divmain divmain force-pushed the divmain/ssr-compiler branch 2 times, most recently from 63f15e3 to 3f1677a Compare February 10, 2024 00:45
Copy link
Contributor

@wjhsf wjhsf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to iterate on this a lot before we release it, should we have it live on a feature branch instead of master?

packages/@lwc/ssr-compiler/src/compile-js/index.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/estree/validators.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/estree/builders.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-runtime/src/index.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-runtime/src/index.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-runtime/src/index.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-runtime/src/index.ts Outdated Show resolved Hide resolved
packages/@lwc/ssr-compiler/src/compile-template/context.ts Outdated Show resolved Hide resolved
} else if (typeof value === 'boolean') {
return [bYield(b.literal(` ${name}`))];
}
throw new Error(`Unknown attr/prop literal: ${type}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will never reach this. value is of type stirng | boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. I think I like having the guard for cases when we're getting lazy about typing and we're casting or using any somewhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fun type guard hack you can do here is

const _valueTypeCheck: never = value

You can only assign never to never, so if the type of value or the preceding conditionals ever change, then this becomes a type error, so you can catch it sooner!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too late now, but it's a bit weird that the error is an unknown value, but the message is refers to type. type is always "Literal", so the error message wouldn't be super helpful here if we accidentally pass, for example, a numeric literal or something.

@divmain divmain force-pushed the divmain/ssr-compiler branch 2 times, most recently from 49a3436 to 5b2f725 Compare April 25, 2024 07:37
@divmain divmain merged commit 6f34189 into master Apr 25, 2024
10 checks passed
@divmain divmain deleted the divmain/ssr-compiler branch April 25, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants