-
Notifications
You must be signed in to change notification settings - Fork 400
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
Conversation
...ages/@lwc/engine-server/src/__tests__/fixtures/export-default-component/modules/x/cmp/cmp.js
Outdated
Show resolved
Hide resolved
...es/@lwc/engine-server/src/__tests__/fixtures/export-default-component/modules/x/cmp/cmp.html
Outdated
Show resolved
Hide resolved
} else { | ||
node.id = b.identifier('DefaultComponentName'); | ||
state.lwcClassName = 'DefaultComponentName'; | ||
} |
There was a problem hiding this comment.
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:
lwc/packages/@lwc/babel-plugin-component/src/component.ts
Lines 38 to 45 in fb163f7
function needsComponentRegistration(path: DeclarationPath) { | |
return ( | |
(path.isIdentifier() && path.node.name !== 'undefined' && path.node.name !== 'null') || | |
path.isCallExpression() || | |
path.isClassDeclaration() || | |
path.isConditionalExpression() | |
); | |
} |
lwc/packages/@lwc/babel-plugin-component/src/decorators/index.ts
Lines 289 to 295 in fb163f7
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 @api
s 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.
@@ -0,0 +1,7 @@ | |||
export const FancyMixin = Class => { | |||
return class extends Class { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ๐
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Details
Fixes #4938.
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item