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

fix(ssr): only default-exported classes are LWC components @W-17312358@ #4942

Merged
merged 7 commits into from
Nov 26, 2024
Merged
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
@@ -0,0 +1,4 @@
<x-cmp>
<template shadowrootmode="open">
</template>
</x-cmp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-cmp';
export { default } from 'x/cmp';
export * from 'x/cmp';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
This template isn't actually used because `export {Component as default}` isn't recognized as an LWC component.
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { LightningElement } from 'lwc';

class Component extends LightningElement {}
export {Component as default}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<x-cmp>
<template shadowrootmode="open">
As the children proclaim, you only live once!
</template>
</x-cmp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-cmp';
export { default } from 'x/cmp';
export * from 'x/cmp';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
As the children proclaim, you only live once!
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LightningElement } from 'lwc';

export default class Component extends LightningElement {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<x-cmp>
<template shadowrootmode="open">
As the children proclaim, you only live once!
</template>
</x-cmp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-cmp';
export { default } from 'x/cmp';
export * from 'x/cmp';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
As the children proclaim, you only live once!
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LightningElement } from 'lwc';

export default class extends LightningElement {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<x-cmp>
<template shadowrootmode="open">
As the children proclaim, you only live once!
</template>
</x-cmp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-cmp';
export { default } from 'x/cmp';
export * from 'x/cmp';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
As the children proclaim, you only live once!
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { LightningElement } from 'lwc';

class Component extends LightningElement {}
export default Component;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<x-component data-lwc-host-mutated="data-yolo" data-yolo>
<template shadowrootmode="open">
yay ook ook ook
</template>
</x-component>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-component';
export { default } from 'x/component';
export * from 'x/component';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
{something} {data}
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { LightningElement } from 'lwc'
import { FancyMixin } from 'x/mixinSuperclass'
import { NotDefault } from 'x/notLwcClass'
import Superless from 'x/superless'

export default class extends FancyMixin(LightningElement) {
something = new NotDefault().prop
data = new Superless().method()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const FancyMixin = Class => {
return class extends Class {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: What happens if this mixin has decorated props?

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually do that in production ๐Ÿ˜†

Copy link
Collaborator

Choose a reason for hiding this comment

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

connectedCallback() {
this.setAttribute('data-yolo', '')
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class NotLightningElement {}
export class NotDefault extends NotLightningElement {
prop = 'yay'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Base {}

class NotExported extends Base {
static value = 'ook ook ook'
}

export default class {
method() {
return NotExported.value
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const expectedFailures = new Set([
'comments-text-preserve-off/index.js',
'dynamic-slots/index.js',
'empty-text-with-comments-non-static-optimized/index.js',
'exports/component-as-default/index.js',
'if-conditional-slot-content/index.js',
'known-boolean-attributes/default-def-html-attributes/static-on-component/index.js',
'render-dynamic-value/index.js',
Expand Down
27 changes: 17 additions & 10 deletions packages/@lwc/ssr-compiler/src/compile-js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,23 @@ const visitors: Visitors = {
},
ClassDeclaration(path, state) {
const { node } = path;
if (!node?.superClass) {
return;
}
// Assume everything with a superclass is an LWC component
state.isLWC = true;
if (node.id) {
state.lwcClassName = node.id.name;
} else {
node.id = b.identifier('DefaultComponentName');
state.lwcClassName = 'DefaultComponentName';
if (
node?.superClass &&
// export default class extends LightningElement {}
(is.exportDefaultDeclaration(path.parentPath) ||
// class Cmp extends LightningElement {}; export default Cmp
path.scope
?.getBinding(node.id.name)
?.references.some((ref) => is.exportDefaultDeclaration(ref.parent)))
) {
// If it's a default-exported class with a superclass, then it's an LWC component!
state.isLWC = true;
if (node.id) {
state.lwcClassName = node.id.name;
} else {
node.id = b.identifier('DefaultComponentName');
state.lwcClassName = 'DefaultComponentName';
}
Copy link
Collaborator

@nolanlawson nolanlawson Nov 25, 2024

Choose a reason for hiding this comment

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

There is actually a lot of logic that the babel-plugin-component currently does to try to distinguish LightningElement classes from non-LE classes:

function needsComponentRegistration(path: DeclarationPath) {
return (
(path.isIdentifier() && path.node.name !== 'undefined' && path.node.name !== 'null') ||
path.isCallExpression() ||
path.isClassDeclaration() ||
path.isConditionalExpression()
);
}

if (
node.superClass === null &&
isAPIFeatureEnabled(
APIFeature.SKIP_UNNECESSARY_REGISTER_DECORATORS,
getAPIVersionFromNumber(state.opts.apiVersion)
)
) {

The first controls registerComponent โ€“ if a component is not registered, then we'll throw an exception at runtime.

The second controls whether decorators are transformed.

I guess at the end of the day we only have to be 1-to-1 compatible if 1) a compiler error is not going to be thrown somewhere, and 2) the ssr-compiler is stricter than the pre-existing toolchain. (#1 is because, on core, components will be compiled both for SSR and CSR, so if CSR throws then we are fine. Dangling @apis on non-LE classes will throw eventually because decorators are not supported natively in Babel yet.)

So I guess I am saying this is fine, but we will probably keep discovering edge cases as we go.

}
},
PropertyDefinition(path, state) {
Expand Down