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.

Sign up for GitHub

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

Jump to bottom

fix: make @api decorator spec compliant #572

Merged
merged 2 commits into from
Aug 12, 2018
Merged

fix: make @api decorator spec compliant #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8289b5a
fix: make @api decorator spec compliant
Aug 12, 2018
074a1e8
fix: removed unused comment
Aug 12, 2018
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
4 changes: 2 additions & 2 deletions packages/babel-plugin-transform-lwc-class/src/__tests__/api-decorator.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('Transform property', () => {
}
Test.publicProps = {
publicAccessor: {
config: 1
config: 3
}
};`
}});
Expand Down Expand Up @@ -164,7 +164,7 @@ describe('Transform property', () => {
}
Test.publicProps = {
publicAccessor: {
config: 2
config: 3
}
};`
}});
Expand Down
23 changes: 23 additions & 0 deletions packages/babel-plugin-transform-lwc-class/src/decorators/api/transform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,21 @@ function getPropertyBitmask(type) {
}
}

function getSiblingGetSetPair(propertyPath, propertyName, type) {
const siblingType = type === 'getter' ? 'set' : 'get';
const klassBody = propertyPath.parentPath.get('body');
const siblingNode = klassBody.find((classMethodPath) => (
classMethodPath !== propertyPath &&
classMethodPath.isClassMethod({ kind: siblingType }) &&
classMethodPath.node.key.name === propertyName)
);

if (siblingNode) {
const decoratorType = siblingType === 'get' ? DECORATOR_TYPES.GETTER :DECORATOR_TYPES.SETTER;
return { type: decoratorType, path: siblingNode };
}
}

/** Returns the public props configuration of a class based on a list decorators. */
function computePublicPropsConfig(decorators) {
return decorators.reduce((acc, { path, type }) => {
Expand All @@ -32,6 +47,14 @@ function computePublicPropsConfig(decorators) {
}

acc[propertyName].config |= getPropertyBitmask(type);

// With the latest decorator spec a decorator only need to be in one of the getter/setter pair
// We need to add the proper bitmask for the sibling getter/setter if exists
const siblingPair = getSiblingGetSetPair(property, propertyName, type);
if (siblingPair) {
acc[propertyName].config |= getPropertyBitmask(siblingPair.type);
Copy link
Member

Choose a reason for hiding this comment

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

What about completely removing the bitmask? We are not using it's capability, all the public properties should have a getter and a setter.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean that we don't use it? We do use it in decorators/api.ts:

    const config = (!isUndefined(meta) && hasOwnProperty.call(target, 'publicProps') && hasOwnProperty.call(meta, propName)) ? meta[propName].config : 0;

Eventually we will be able to remove it, but for now, we can't.

}

return acc;
}, {});
}
Expand Down
7 changes: 2 additions & 5 deletions packages/babel-plugin-transform-lwc-class/src/decorators/api/validate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ function validatePropertyName(property) {
}

function validateSingleApiDecoratorOnSetterGetterPair(decorators) {
decorators.filter(decorator => (
isApiDecorator(decorator) &&
decorator.type === DECORATOR_TYPES.SETTER
)).forEach(({ path }) => {
decorators.filter(decorator => (isApiDecorator(decorator) && decorator.type === DECORATOR_TYPES.SETTER)).forEach(({ path }) => {
const name = path.parentPath.get('key.name').node;

const associatedGetter = decorators.find(decorator => (
Expand All @@ -73,7 +70,7 @@ function validateSingleApiDecoratorOnSetterGetterPair(decorators) {
));

if (associatedGetter) {
console.warn(`\`@api get ${name}\` and \`@api set ${name}\` detected in class declaration. Only the getter needs to be decorated with @api.`);
console.warn(`\`@api get ${name}\` and \`@api set ${name}\` detected in class declaration. Only one of the two needs to be decorated with @api.`);
}
});
}
Expand Down