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

chore: use __lwc for transmogrified function names #5278

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('component compilation', () => {
`;
const filename = path.resolve('component.js');
const { code } = compileComponentForSSR(src, filename, {});
expect(code).toContain('import tmpl from "./component.html"');
expect(code).toContain('import __lwcTmpl from "./component.html"');
});
test('explicit templates imports do not use full file paths', () => {
const src = `
Expand Down Expand Up @@ -59,7 +59,7 @@ describe('component compilation', () => {
`;
const filename = path.resolve('component.ts');
const { code } = compileComponentForSSR(src, filename, {});
expect(code).toContain('import tmpl from "./component.html"');
expect(code).toContain('import __lwcTmpl from "./component.html"');
});

describe('wire decorator', () => {
Expand Down
32 changes: 16 additions & 16 deletions packages/@lwc/ssr-compiler/src/__tests__/transmogrify.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { transmogrify } from '../transmogrify';
import type { Program as EsProgram } from 'estree';

const COMPILED_CMP = `
async function* tmpl(props, attrs, slottedContent, Cmp, instance) {
async function* __lwcTmpl(props, attrs, slottedContent, Cmp, instance) {
yield "<p";
yield stylesheetScopeTokenClass;
yield ">Hello</p>";
Expand All @@ -27,7 +27,7 @@ const COMPILED_CMP = `
}
}
const __REFLECTED_PROPS__ = [];
async function* generateMarkup(tagName, props, attrs, slotted) {
async function* __lwcGenerateMarkup(tagName, props, attrs, slotted) {
attrs = attrs ?? ({});
const instance = new Basic({
tagName: tagName.toUpperCase()
Expand Down Expand Up @@ -65,10 +65,10 @@ describe('transmogrify', () => {
});

describe('in sync mode', () => {
test('generateMarkup is transformed into sync mode', () => {
expect(COMPILED_CMP_SYNC).not.toContain('async function* generateMarkup');
expect(COMPILED_CMP_SYNC).not.toContain('async function generateMarkup');
expect(COMPILED_CMP_SYNC).toContain('function generateMarkup($$emit');
test('__lwcGenerateMarkup is transformed into sync mode', () => {
expect(COMPILED_CMP_SYNC).not.toContain('async function* __lwcGenerateMarkup');
expect(COMPILED_CMP_SYNC).not.toContain('async function __lwcGenerateMarkup');
expect(COMPILED_CMP_SYNC).toContain('function __lwcGenerateMarkup($$emit');

expect(COMPILED_CMP_SYNC).not.toContain('yield* renderAttrs');
expect(COMPILED_CMP_SYNC).toContain('renderAttrs($$emit');
Expand All @@ -78,10 +78,10 @@ describe('transmogrify', () => {
expect(COMPILED_CMP_SYNC).toContain('$$emit(">")');
});

test('tmpl is transformed into sync mode', () => {
expect(COMPILED_CMP_SYNC).not.toContain('async function* tmpl');
expect(COMPILED_CMP_SYNC).not.toContain('async function tmpl');
expect(COMPILED_CMP_SYNC).toContain('function tmpl($$emit');
test('__lwcTmpl is transformed into sync mode', () => {
expect(COMPILED_CMP_SYNC).not.toContain('async function* __lwcTmpl');
expect(COMPILED_CMP_SYNC).not.toContain('async function __lwcTmpl');
expect(COMPILED_CMP_SYNC).toContain('function __lwcTmpl($$emit');

expect(COMPILED_CMP_SYNC).not.toContain('yield "<p"');
expect(COMPILED_CMP_SYNC).toContain('$$emit("<p")');
Expand All @@ -102,9 +102,9 @@ describe('transmogrify', () => {
});

describe('in async mode', () => {
test('generateMarkup is transformed into async mode', () => {
expect(COMPILED_CMP_ASYNC).not.toContain('async function* generateMarkup');
expect(COMPILED_CMP_ASYNC).toContain('async function generateMarkup($$emit');
test('__lwcGenerateMarkup is transformed into async mode', () => {
expect(COMPILED_CMP_ASYNC).not.toContain('async function* __lwcGenerateMarkup');
expect(COMPILED_CMP_ASYNC).toContain('async function __lwcGenerateMarkup($$emit');

expect(COMPILED_CMP_ASYNC).not.toContain('yield* renderAttrs');
expect(COMPILED_CMP_ASYNC).toContain('renderAttrs($$emit');
Expand All @@ -114,9 +114,9 @@ describe('transmogrify', () => {
expect(COMPILED_CMP_ASYNC).toContain('$$emit(">")');
});

test('tmpl is transformed into async mode', () => {
expect(COMPILED_CMP_ASYNC).not.toContain('async function* tmpl');
expect(COMPILED_CMP_ASYNC).toContain('async function tmpl($$emit');
test('__lwcTmpl is transformed into async mode', () => {
expect(COMPILED_CMP_ASYNC).not.toContain('async function* __lwcTmpl');
expect(COMPILED_CMP_ASYNC).toContain('async function __lwcTmpl($$emit');

expect(COMPILED_CMP_ASYNC).not.toContain('yield "<p"');
expect(COMPILED_CMP_ASYNC).toContain('$$emit("<p")');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const bGenerateMarkup = esTemplate`
configurable: false,
enumerable: false,
writable: false,
value: async function* generateMarkup(
value: async function* __lwcGenerateMarkup(
// The $$emit function is magically inserted here
tagName,
props,
Expand Down Expand Up @@ -139,7 +139,7 @@ export function addGenerateMarkupFunction(
let exposeTemplateBlock: IfStatement | null = null;
if (!tmplExplicitImports) {
const defaultTmplPath = `./${pathParse(filename).name}.html`;
const tmplVar = b.identifier('tmpl');
const tmplVar = b.identifier('__lwcTmpl');
program.body.unshift(bImportDeclaration({ default: tmplVar.name }, defaultTmplPath));
program.body.unshift(
bImportDeclaration({ SYMBOL__DEFAULT_TEMPLATE: '__SYMBOL__DEFAULT_TEMPLATE' })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ export function addScopeTokenDeclarations(
bHasScopedStylesheetsDeclaration()
);

program.body.push(...tmplAssignmentBlock(b.identifier('tmpl')));
program.body.push(...tmplAssignmentBlock(b.identifier('__lwcTmpl')));
}
2 changes: 1 addition & 1 deletion packages/@lwc/ssr-compiler/src/compile-template/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type {

// TODO [#4663]: Render mode mismatch between template and compiler should throw.
const bExportTemplate = esTemplate`
export default async function* tmpl(
export default async function* __lwcTmpl(
// This is where $$emit comes from
shadowSlottedContent,
lightSlottedContent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import type { TransformerContext } from '../../types';

const bGenerateSlottedContent = esTemplateWithYield`
const shadowSlottedContent = ${/* hasShadowSlottedContent */ is.literal}
? async function* generateSlottedContent(contextfulParent) {
? async function* __lwcGenerateSlottedContent(contextfulParent) {
// The 'contextfulParent' variable is shadowed here so that a contextful relationship
// is established between components rendered in slotted content & the "parent"
// component that contains the <slot>.
Expand Down Expand Up @@ -72,11 +72,11 @@ const bGenerateSlottedContent = esTemplateWithYield`
${/* scoped slot addLightContent statements */ is.expressionStatement}
`<EsStatement[]>;

// Note that this function name (`generateSlottedContent`) does not need to be scoped even though
// Note that this function name (`__lwcGenerateSlottedContent`) does not need to be scoped even though
// it may be repeated multiple times in the same scope, because it's a function _expression_ rather
// than a function _declaration_, so it isn't available to be referenced anywhere.
const bAddSlottedContent = esTemplate`
addSlottedContent(${/* slot name */ is.expression} ?? "", async function* generateSlottedContent(contextfulParent, ${
addSlottedContent(${/* slot name */ is.expression} ?? "", async function* __lwcGenerateSlottedContent(contextfulParent, ${
/* scoped slot data variable */ isNullableOf(is.identifier)
}, slotAttributeValue) {
// FIXME: make validation work again
Expand Down
27 changes: 10 additions & 17 deletions packages/@lwc/ssr-compiler/src/transmogrify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,24 @@ interface TransmogrificationState {
export type Visitors = Parameters<typeof traverse<Node, TransmogrificationState>>[1];

const EMIT_IDENT = b.identifier('$$emit');
/** Function names that may be transmogrified. All should start with `__lwc`. */
// Rollup may rename variables to prevent shadowing. When it does, it uses the format `foo$0`, `foo$1`, etc.
const TMPL_FN_PATTERN = /tmpl($\d+)?/;
const GEN_MARKUP_OR_GEN_SLOTTED_CONTENT_PATTERN =
/(?:generateMarkup|generateSlottedContent)($\d+)?/;
const TRANSMOGRIFY_TARGET = /^__lwc(GenerateMarkup|GenerateSlottedContent|Tmpl)(?:\$\d+)?$/;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Having two regex to check for three nearly identical things seemed silly.


const isWithinFn = (pattern: RegExp, nodePath: NodePath): boolean => {
const isWithinFn = (nodePath: NodePath): boolean => {
const { node } = nodePath;
if (!node) {
return false;
}
if (
(node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression') &&
node.id &&
pattern.test(node.id.name)
TRANSMOGRIFY_TARGET.test(node.id.name)
) {
return true;
}
if (nodePath.parentPath) {
return isWithinFn(pattern, nodePath.parentPath);
return isWithinFn(nodePath.parentPath);
}
return false;
};
Expand All @@ -57,10 +56,7 @@ const visitors: Visitors = {
// Component authors might conceivably use async generator functions in their own code. Therefore,
// when traversing & transforming written+generated code, we need to disambiguate generated async
// generator functions from those that were written by the component author.
if (
!isWithinFn(GEN_MARKUP_OR_GEN_SLOTTED_CONTENT_PATTERN, path) &&
!isWithinFn(TMPL_FN_PATTERN, path)
) {
if (!isWithinFn(path)) {
return;
}
node.generator = false;
Expand All @@ -76,10 +72,7 @@ const visitors: Visitors = {
// Component authors might conceivably use generator functions within their own code. Therefore,
// when traversing & transforming written+generated code, we need to disambiguate generated yield
// expressions from those that were written by the component author.
if (
!isWithinFn(TMPL_FN_PATTERN, path) &&
!isWithinFn(GEN_MARKUP_OR_GEN_SLOTTED_CONTENT_PATTERN, path)
) {
if (!isWithinFn(path)) {
return;
}

Expand Down Expand Up @@ -152,7 +145,7 @@ const visitors: Visitors = {
* Is compiled into the following JavaScript, intended for execution during SSR & stripped down
* for the purposes of this example:
*
* async function* tmpl(props, attrs, slottedContent, Cmp, instance) {
* async function* __lwcTmpl(props, attrs, slottedContent, Cmp, instance) {
* yield '<div>foobar</div>';
* const childProps = {};
* const childAttrs = {};
Expand All @@ -161,7 +154,7 @@ const visitors: Visitors = {
*
* When transmogrified in async-mode, the above generated template function becomes the following:
*
* async function tmpl($$emit, props, attrs, slottedContent, Cmp, instance) {
* async function __lwcTmpl($$emit, props, attrs, slottedContent, Cmp, instance) {
* $$emit('<div>foobar</div>');
* const childProps = {};
* const childAttrs = {};
Expand All @@ -170,7 +163,7 @@ const visitors: Visitors = {
*
* When transmogrified in sync-mode, the template function becomes the following:
*
* function tmpl($$emit, props, attrs, slottedContent, Cmp, instance) {
* function __lwcTmpl($$emit, props, attrs, slottedContent, Cmp, instance) {
* $$emit('<div>foobar</div>');
* const childProps = {};
* const childAttrs = {};
Expand Down