Skip to content

Commit

Permalink
fix(compiler): don't allow shadowRoot getter to avoid hydration issues (
Browse files Browse the repository at this point in the history
#5912)

* fix(compiler): don't allow shadowRoot getter to avoid hydration issues

* remove magic string

* prettier
  • Loading branch information
christian-bromann authored Aug 1, 2024
1 parent 5f8c969 commit 5dd4f7f
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 0 deletions.
48 changes: 48 additions & 0 deletions src/compiler/transformers/static-to-meta/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ import { parseStringLiteral } from './string-literal';
import { parseStaticStyles } from './styles';
import { parseStaticWatchers } from './watchers';

const BLACKLISTED_COMPONENT_METHODS = [
/**
* If someone would define a getter called "shadowRoot" on a component
* this would cause issues when Stencil tries to hydrate the component.
*/
'shadowRoot',
];

/**
* Given a {@see ts.ClassDeclaration} which represents a Stencil component
* class declaration, parse and format various pieces of data about static class
Expand Down Expand Up @@ -148,6 +156,8 @@ export const parseStaticComponentMeta = (
};

const visitComponentChildNode = (node: ts.Node) => {
validateComponentMembers(node);

if (ts.isCallExpression(node)) {
parseCallExpression(cmp, node);
} else if (ts.isStringLiteral(node)) {
Expand Down Expand Up @@ -176,6 +186,44 @@ export const parseStaticComponentMeta = (
return cmpNode;
};

const validateComponentMembers = (node: ts.Node) => {
/**
* validate if:
*/
if (
/**
* the component has a getter called "shadowRoot"
*/
ts.isGetAccessorDeclaration(node) &&
ts.isIdentifier(node.name) &&
typeof node.name.escapedText === 'string' &&
BLACKLISTED_COMPONENT_METHODS.includes(node.name.escapedText) &&
/**
* the parent node is a class declaration
*/
ts.isClassDeclaration(node.parent)
) {
const propName = node.name.escapedText;
const decorator = ts.getDecorators(node.parent)[0];
/**
* the class is actually a Stencil component, has a decorator with a property named "tag"
*/
if (
ts.isCallExpression(decorator.expression) &&
decorator.expression.arguments.length === 1 &&
ts.isObjectLiteralExpression(decorator.expression.arguments[0]) &&
decorator.expression.arguments[0].properties.some(
(prop) => ts.isPropertyAssignment(prop) && prop.name.getText() === 'tag',
)
) {
const componentName = node.parent.name.getText();
throw new Error(
`The component "${componentName}" has a getter called "${propName}". This getter is reserved for use by Stencil components and should not be defined by the user.`,
);
}
}
};

const parseVirtualProps = (docs: d.CompilerJsDoc) => {
return docs.tags
.filter(({ name }) => name === 'virtualProp')
Expand Down
44 changes: 44 additions & 0 deletions src/compiler/transformers/test/parse-component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,48 @@ describe('parse component', () => {

expect(t.componentClassName).toBe('CmpA');
});

it('can not have shadowRoot getter', () => {
let error: Error | undefined;
try {
transpileModule(`
@Component({
tag: 'cmp-a'
})
export class CmpA {
get shadowRoot() {
return this;
}
}
`);
} catch (err: unknown) {
error = err as Error;
}

expect(error.message).toContain(
`The component "CmpA" has a getter called "shadowRoot". This getter is reserved for use by Stencil components and should not be defined by the user.`,
);
});

it('ignores shadowRoot getter in unrelated class', () => {
const t = transpileModule(`
@Component({
tag: 'cmp-a'
})
export class CmpA {
// use a better name for the getter
get elementShadowRoot() {
return this;
}
}
export class Unrelated {
get shadowRoot() {
return this;
}
}
`);

expect(t.componentClassName).toBe('CmpA');
});
});

0 comments on commit 5dd4f7f

Please sign in to comment.