From bf2bfa7e358df3106a5dd727d6a25275a79fc7bc Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 19 Mar 2019 17:52:48 -0700 Subject: [PATCH 1/4] fix(babel-plugin-component): improve class member decorator validation --- .../src/__tests__/api-decorator.spec.js | 115 ++++++++++++++++++ .../src/decorators/api/validate.js | 36 +++--- 2 files changed, 134 insertions(+), 17 deletions(-) diff --git a/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js b/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js index a8eb45e071..12ec635d90 100644 --- a/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js +++ b/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js @@ -164,6 +164,121 @@ describe('Transform property', () => { } ); + pluginTest( + 'single @api getter on one class member should not conflict with @api setter on another', + ` + import { api } from 'lwc'; + export default class Test { + @api + get first() { return null; } + + @api + get second() { + return this.s; + } + set second(value) { + this.s = value; + } + } + `, + { + output: { + code: ` + import { registerDecorators as _registerDecorators } from "lwc"; + import _tmpl from "./test.html"; + import { registerComponent as _registerComponent } from "lwc"; + + class Test { + get first() { + return null; + } + + get second() { + return this.s; + } + + set second(value) { + this.s = value; + } + } + + _registerDecorators(Test, { + publicProps: { + first: { + config: 1 + }, + second: { + config: 3 + } + } + }); + + export default _registerComponent(Test, { + tmpl: _tmpl + }); + `, + }, + } + ); + + pluginTest( + '@api setter on one class member should not conflict with @api getter on another', + ` + import { api } from 'lwc'; + export default class Test { + @api + set first(value) {} + get first() {} + + @api + get second() { + return this.s; + } + set second (value) { + this.s = value; + } + } + `, + { + output: { + code: ` + import { registerDecorators as _registerDecorators } from "lwc"; + import _tmpl from "./test.html"; + import { registerComponent as _registerComponent } from "lwc"; + + class Test { + set first(value) {} + + get first() {} + + get second() { + return this.s; + } + + set second(value) { + this.s = value; + } + } + + _registerDecorators(Test, { + publicProps: { + first: { + config: 3 + }, + second: { + config: 3 + } + } + }); + + export default _registerComponent(Test, { + tmpl: _tmpl + }); + `, + }, + } + ); + pluginTest( 'transform pairs of setter and getter', ` diff --git a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js index b79927f075..a69513c0a1 100644 --- a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js +++ b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js @@ -81,26 +81,27 @@ function validatePropertyName(property) { } function validateSingleApiDecoratorOnSetterGetterPair(decorators) { - decorators - .filter(decorator => isApiDecorator(decorator) && decorator.type === DECORATOR_TYPES.SETTER) - .forEach(({ path }) => { - const parentPath = path.parentPath; - const name = parentPath.get('key.name').node; - - const associatedGetter = decorators.find( - decorator => - isApiDecorator(decorator) && - decorator.type === DECORATOR_TYPES.GETTER && - parentPath.get('key.name').node === name - ); - - if (associatedGetter) { - throw generateError(parentPath, { + // keep track of visited class methods + const visitedMethods = new Set(); + + decorators.forEach(decorator => { + const { path, type } = decorator; + + // since we are validating get/set we only look at @api methods + if (isApiDecorator(decorator) && type !== DECORATOR_TYPES.PROPERTY) { + const methodPath = path.parentPath; + const methodName = methodPath.get('key.name').node; + + if (visitedMethods.has(methodName)) { + throw generateError(methodPath, { errorInfo: DecoratorErrors.SINGLE_DECORATOR_ON_SETTER_GETTER_PAIR, - messageArgs: [name], + messageArgs: [methodName], }); } - }); + + visitedMethods.add(methodName); + } + }); } function validateUniqueness(decorators) { @@ -145,6 +146,7 @@ module.exports = function validate(klass, decorators) { } }); + //console.log('---> decorators: ', decorators); validateSingleApiDecoratorOnSetterGetterPair(decorators); validateUniqueness(decorators); }; From 9d060fc1ba0e92429578d7a43b3f56e0dbad132d Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 19 Mar 2019 17:58:50 -0700 Subject: [PATCH 2/4] wip: change decorator setter/getter condition --- .../babel-plugin-component/src/decorators/api/validate.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js index a69513c0a1..b0e03ad723 100644 --- a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js +++ b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js @@ -88,7 +88,10 @@ function validateSingleApiDecoratorOnSetterGetterPair(decorators) { const { path, type } = decorator; // since we are validating get/set we only look at @api methods - if (isApiDecorator(decorator) && type !== DECORATOR_TYPES.PROPERTY) { + if ( + isApiDecorator(decorator) && + (type === DECORATOR_TYPES.GETTER || type === DECORATOR_TYPES.SETTER) + ) { const methodPath = path.parentPath; const methodName = methodPath.get('key.name').node; From 4e8b08355ec6e8ccbf764d3bc095a1edba374da4 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 19 Mar 2019 18:01:56 -0700 Subject: [PATCH 3/4] test: correct test code --- .../babel-plugin-component/src/__tests__/api-decorator.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js b/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js index 12ec635d90..25310ac4ec 100644 --- a/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js +++ b/packages/@lwc/babel-plugin-component/src/__tests__/api-decorator.spec.js @@ -172,10 +172,10 @@ describe('Transform property', () => { @api get first() { return null; } - @api get second() { return this.s; } + @api set second(value) { this.s = value; } From ca7f7b798190f68d76a3ac20ad45f7495bebc06b Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 20 Mar 2019 08:36:34 -0700 Subject: [PATCH 4/4] wip: remove commented comment --- .../@lwc/babel-plugin-component/src/decorators/api/validate.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js index b0e03ad723..0f8480bb0a 100644 --- a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js +++ b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.js @@ -149,7 +149,6 @@ module.exports = function validate(klass, decorators) { } }); - //console.log('---> decorators: ', decorators); validateSingleApiDecoratorOnSetterGetterPair(decorators); validateUniqueness(decorators); };