Skip to content

Commit

Permalink
refactor(wire-service): wire decorator reform
Browse files Browse the repository at this point in the history 
Squashed commit of the following:
commit 0f00e08
Author: Jose David Rodriguez <jodarove@gmail.com>
Date:   Thu Sep 19 23:13:15 2019 -0700

    refactor(wire-service): wire decorator reform

    Squashed commit of the following:

    commit fb136b7
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Thu Aug 29 23:59:16 2019 -0400

        fix(wire-service): does not accept adapter id to be a symbol

    commit 91c2d97
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Thu Aug 29 15:04:19 2019 -0400

        fix(engine): tests and karma tests fixes

    commit 719c41e
    Merge: 3cdf650 8e5035e
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Sat Aug 31 14:31:07 2019 -0400

        Merge branch 'master' into caridy/wire-reform-2

    commit 8e5035e
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Fri Aug 30 22:25:49 2019 -0400

        refactor: hidden fields instead of internal fields (2) (#1485)

        * chore: package-unique keys for engine and synthetic shadow

        * refactor: remove internal fields in favor of hidden fields

        * fix: avoid returning boolean false when field undefined

        * test: use hidden fields instead of internal fields

        * refactor(synthetic-shadow): hidden fields instead of internal fields

        * chore(synthetic-shadow): "unique" string keys

        * fix(engine): issue 1299 second attempt

        * fix(engine): removing hasOwnProperty check

        * chore: revert changes for vm access by getComponentConstructor

    commit 9a7f822
    Author: Ravi Jayaramappa <ravi.jayaramappa@salesforce.com>
    Date:   Thu Aug 29 14:05:57 2019 -0700

        fix: cloneNode() default behavior should match spec (#1480)

    commit 3cdf650
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Thu Aug 29 05:40:52 2019 -0400

        fix(engine): context to work as expected

    commit 2224bd2
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Thu Aug 29 04:38:45 2019 -0400

        fix(engine): karma tests for context providers

    commit 8b6c978
    Merge: d02243b 5d5f7af
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Thu Aug 29 04:33:58 2019 -0400

        Merge branch 'caridy/wire-reform-2' of github.com:salesforce/lwc into caridy/wire-reform-2

    commit 5d5f7af
    Author: Jose David Rodriguez <jodarove@gmail.com>
    Date:   Fri Aug 30 13:30:34 2019 -0700

        fix: observable-fields

    commit a901a64
    Author: Jose David Rodriguez <jodarove@gmail.com>
    Date:   Thu Aug 29 22:41:50 2019 -0700

        wip: wire register is broken it was making karma fail

    commit d02243b
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Wed Aug 28 07:46:56 2019 -0400

        fix(engine): remove unnecessary comment

    commit c9ad2c5
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Wed Aug 28 02:59:29 2019 -0400

        refactor(engine): context provider options

    commit 6bcf0be
    Author: Caridy Patiño <caridy@gmail.com>
    Date:   Tue Jul 16 23:03:56 2019 -0400

        fix(engine): @wire() protocol reform RFC

fix: rebase issues

fix: wire-reform tests (#1524)

* fix: invalid syntax error on invalid wire param value

When a wire parameter has invalid value (eg: foo..bar, foo.bar[3]) it should (ideally) resolve in a compilation error during validation phase.

This is not possible due that platform components may have a param definition which is invalid but passes compilation, and throwing at compile time would break such components.

In such cases where the param does not have proper notation, the config generated will use the bracket notation to match the current behavior (that most likely end up resolving that param as undefined).

* fix: tests for wire reactive parameters

* fix: unit test register and WireAdapter

* fix: backward compability when adding config listener

* fix: adding general tests for wire adapter

* wip: address pr comments and lint errors

fix: reactivity of wire params (#1526)

* fix: reactivity of wire params

* wip: address review comments

and run performance

* wip: trigger perf measures

* fix: remove valueMutated pruning condition

* fix: add componentValueObserved for consistency

* fix: remove tf because of wired properties being track

before the wire reform.

* fix: ensure component rerender in compat

* fix: tf because registerDecorators happens before registerAdapter

* Revert "fix: tf because registerDecorators happens before registerAdapter"

This reverts commit b2d1a89.

* fix: wire integration tests

fix: rebase issues

fix: remove tests because wire defs is not part of the public api

of getComponentDef

fix: add test for inherited methods

fix: context in targets hierarchy
  • Loading branch information
@jodarove
jodarove committed Oct 8, 2019
1 parent 03231bb commit 00b56c9
Show file tree
Hide file tree
Showing 68 changed files with 2,331 additions and 2,975 deletions.
106 changes: 106 additions & 0 deletions packages/@lwc/babel-plugin-component/src/__tests__/wire-decorator.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,112 @@ describe('Transform property', () => {
}
);

pluginTest(
"config function should use bracket notation for param when it's definition has invalid identifier as segment",
`
import { api, wire } from 'lwc';
import { getFoo } from 'data-service';
export default class Test {
@wire(getFoo, { key1: "$prop1.a b", key2: "$p1.p2" })
wiredProp;
}
`,
{
output: {
code: `
import { registerDecorators as _registerDecorators } from "lwc";
import _tmpl from "./test.html";
import { registerComponent as _registerComponent } from "lwc";
import { getFoo } from "data-service";
class Test {
constructor() {
this.wiredProp = void 0;
}
}
_registerDecorators(Test, {
wire: {
wiredProp: {
adapter: getFoo,
params: {
key1: "prop1.a b",
key2: "p1.p2"
},
static: {},
config: function($cmp) {
let v1 = $cmp["prop1"];
let v2 = $cmp.p1;
return {
key1: v1 != null ? v1["a b"] : undefined,
key2: v2 != null ? v2.p2 : undefined
};
}
}
}
});
export default _registerComponent(Test, {
tmpl: _tmpl
});
`,
},
}
);

pluginTest(
'config function should use bracket notation when param definition has empty segment',
`
import { api, wire } from 'lwc';
import { getFoo } from 'data-service';
export default class Test {
@wire(getFoo, { key1: "$prop1..prop2", key2: ["fixed", 'array']})
wiredProp;
}
`,
{
output: {
code: `
import { registerDecorators as _registerDecorators } from "lwc";
import _tmpl from "./test.html";
import { registerComponent as _registerComponent } from "lwc";
import { getFoo } from "data-service";
class Test {
constructor() {
this.wiredProp = void 0;
}
}
_registerDecorators(Test, {
wire: {
wiredProp: {
adapter: getFoo,
params: {
key1: "prop1..prop2"
},
static: {
key2: ["fixed", "array"]
},
config: function($cmp) {
let v1 = $cmp["prop1"];
return {
key2: ["fixed", "array"],
key1: v1 != null && (v1 = v1[""]) != null ? v1["prop2"] : undefined
};
}
}
}
});
export default _registerComponent(Test, {
tmpl: _tmpl
});
`,
},
}
);

pluginTest(
'throws when wired property is combined with @track',
`
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/babel-plugin-component/src/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const LWC_API_WHITELIST = new Set([
'getComponentDef',
'getComponentConstructor',
'isComponentConstructor',
'createContextProvider',
'readonly',
'register',
'setFeatureFlagForTest',
Expand Down
25 changes: 21 additions & 4 deletions packages/@lwc/babel-plugin-component/src/decorators/wire/transform.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,22 @@ function getGeneratedConfig(t, wiredValue) {
const configBlockBody = [];
const configProps = [];
const generateParameterConfigValue = memberExprPaths => {
// Note: When memberExprPaths ($foo.bar) has an invalid identifier (eg: foo..bar, foo.bar[3])
// it should (ideally) resolve in a compilation error during validation phase.
// This is not possible due that platform components may have a param definition which is invalid
// but passes compilation, and throwing at compile time would break such components.
// In such cases where the param does not have proper notation, the config generated will use the bracket
// notation to match the current behavior (that most likely end up resolving that param as undefined).
const isInvalidMemberExpr = memberExprPaths.some(
maybeIdentifier => !t.isValidES3Identifier(maybeIdentifier)
);
const memberExprPropertyGen = !isInvalidMemberExpr ? t.identifier : t.StringLiteral;

if (memberExprPaths.length === 1) {
return {
configValueExpression: t.memberExpression(
t.identifier(WIRE_CONFIG_ARG_NAME),
t.identifier(memberExprPaths[0])
memberExprPropertyGen(memberExprPaths[0])
),
};
}
Expand All @@ -58,7 +69,8 @@ function getGeneratedConfig(t, wiredValue) {
t.identifier(varName),
t.memberExpression(
t.identifier(WIRE_CONFIG_ARG_NAME),
t.identifier(memberExprPaths[0])
memberExprPropertyGen(memberExprPaths[0]),
isInvalidMemberExpr
)
),
]);
Expand All @@ -70,7 +82,11 @@ function getGeneratedConfig(t, wiredValue) {
const nextPropValue = t.assignmentExpression(
'=',
t.identifier(varName),
t.memberExpression(t.identifier(varName), t.identifier(memberExprPaths[i]))
t.memberExpression(
t.identifier(varName),
memberExprPropertyGen(memberExprPaths[i]),
isInvalidMemberExpr
)
);

conditionTest = t.logicalExpression(
Expand All @@ -85,7 +101,8 @@ function getGeneratedConfig(t, wiredValue) {
conditionTest,
t.memberExpression(
t.identifier(varName),
t.identifier(memberExprPaths[memberExprPaths.length - 1])
memberExprPropertyGen(memberExprPaths[memberExprPaths.length - 1]),
isInvalidMemberExpr
),
t.identifier('undefined')
);
Expand Down
2 changes: 1 addition & 1 deletion packages/@lwc/engine/src/framework/__tests__/error-boundary.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ describe('error boundary component', () => {
}
}
registerDecorators(PreErrorChildContent, {
publicProps: { foo: { config: 1 } },
publicProps: { foo: { config: 3 } },
});
const baseTmpl = compileTemplate(
`
Expand Down
31 changes: 14 additions & 17 deletions packages/@lwc/engine/src/framework/__tests__/html-element.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,10 +494,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { foo: {} },
});

registerDecorators(MyComponent, {
publicProps: { foo: { config: 3 } },
track: { state: 1 },
});

Expand Down Expand Up @@ -550,7 +547,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { role: {} },
publicProps: { role: { config: 3 } },
});
const element = createElement('prop-getter-aria-role', { is: MyComponent });
document.body.appendChild(element);
Expand Down Expand Up @@ -588,7 +585,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { lang: {} },
publicProps: { lang: { config: 3 } },
});

const element = createElement('prop-setter-lang', { is: MyComponent });
Expand Down Expand Up @@ -634,7 +631,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { lang: {} },
publicProps: { lang: { config: 1 } },
});

const element = createElement('prop-getter-lang-imperative', { is: MyComponent });
Expand Down Expand Up @@ -708,7 +705,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { hidden: {} },
publicProps: { hidden: { config: 3 } },
});

const element = createElement('prop-setter-hidden', { is: MyComponent });
Expand Down Expand Up @@ -753,7 +750,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { hidden: {} },
publicProps: { hidden: { config: 1 } },
});

const element = createElement('prop-getter-hidden-imperative', { is: MyComponent });
Expand Down Expand Up @@ -831,7 +828,7 @@ describe('html-element', () => {
}

registerDecorators(MyComponent, {
publicProps: { dir: {} },
publicProps: { dir: { config: 3 } },
});

const element = createElement('prop-setter-dir', { is: MyComponent });
Expand Down Expand Up @@ -877,7 +874,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { dir: {} },
publicProps: { dir: { config: 1 } },
});

const element = createElement('prop-getter-dir-imperative', { is: MyComponent });
Expand Down Expand Up @@ -953,7 +950,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { id: {} },
publicProps: { id: { config: 3 } },
});

const element = createElement('prop-setter-id', { is: MyComponent });
Expand Down Expand Up @@ -999,7 +996,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { id: {} },
publicProps: { id: { config: 1 } },
});

const element = createElement('prop-getter-id-imperative', { is: MyComponent });
Expand Down Expand Up @@ -1077,7 +1074,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { accessKey: {} },
publicProps: { accessKey: { config: 3 } },
});

const element = createElement('prop-setter-accessKey', { is: MyComponent });
Expand Down Expand Up @@ -1124,7 +1121,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { accessKey: {} },
publicProps: { accessKey: { config: 1 } },
});
const element = createElement('prop-getter-accessKey-imperative', {
is: MyComponent,
Expand Down Expand Up @@ -1201,7 +1198,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { title: {} },
publicProps: { title: { config: 3 } },
});
const element = createElement('prop-setter-title', { is: MyComponent });
(element.title = {}), expect(count).toBe(1);
Expand Down Expand Up @@ -1244,7 +1241,7 @@ describe('html-element', () => {
}
}
registerDecorators(MyComponent, {
publicProps: { title: {} },
publicProps: { title: { config: 1 } },
});

const element = createElement('prop-getter-title-imperative', { is: MyComponent });
Expand Down
38 changes: 13 additions & 25 deletions packages/@lwc/engine/src/framework/base-lightning-element.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,15 @@
* shape of a component. It is also used internally to apply extra optimizations.
*/
import {
ArrayReduce,
assert,
create,
defineProperties,
fields,
freeze,
getOwnPropertyNames,
isFalse,
isFunction,
isNull,
isObject,
seal,
isObject,
} from '@lwc/shared';
import { logError } from '../shared/assert';
import { HTMLElementOriginalDescriptors } from './html-properties';
Expand All @@ -38,7 +35,7 @@ import {
import { ViewModelReflection, EmptyObject } from './utils';
import { vmBeingConstructed, isBeingConstructed, isRendering, vmBeingRendered } from './invoker';
import { getComponentVM, VM } from './vm';
import { valueObserved, valueMutated } from '../libs/mutation-tracker';
import { componentValueMutated, componentValueObserved } from './mutation-tracker';
import { dispatchEvent } from '../env/dom';
import { patchComponentWithRestrictions, patchShadowRootWithRestrictions } from './restrictions';
import { unlockAttribute, lockAttribute } from './attributes';
Expand Down Expand Up @@ -93,7 +90,7 @@ function createBridgeToElementDescriptor(
}
return;
}
valueObserved(this, propName);
componentValueObserved(vm, propName);
return get.call(vm.elm);
},
set(this: ComponentInterface, newValue: any) {
Expand All @@ -118,10 +115,8 @@ function createBridgeToElementDescriptor(

if (newValue !== vm.cmpProps[propName]) {
vm.cmpProps[propName] = newValue;
if (isFalse(vm.isDirty)) {
// perf optimization to skip this step if not in the DOM
valueMutated(this, propName);
}

componentValueMutated(vm, propName);
}
return set.call(vm.elm, newValue);
},
Expand Down Expand Up @@ -564,22 +559,15 @@ BaseLightningElementConstructor.prototype = {
},
};

// Typescript is inferring the wrong function type for this particular
// overloaded method: https://github.com/Microsoft/TypeScript/issues/27972
// @ts-ignore type-mismatch
const baseDescriptors: PropertyDescriptorMap = ArrayReduce.call(
getOwnPropertyNames(HTMLElementOriginalDescriptors),
(descriptors: PropertyDescriptorMap, propName: string) => {
descriptors[propName] = createBridgeToElementDescriptor(
propName,
HTMLElementOriginalDescriptors[propName]
);
return descriptors;
},
create(null)
);
export const lightningBasedDescriptors: PropertyDescriptorMap = create(null);
for (const propName in HTMLElementOriginalDescriptors) {
lightningBasedDescriptors[propName] = createBridgeToElementDescriptor(
propName,
HTMLElementOriginalDescriptors[propName]
);
}

defineProperties(BaseLightningElementConstructor.prototype, baseDescriptors);
defineProperties(BaseLightningElementConstructor.prototype, lightningBasedDescriptors);

if (process.env.NODE_ENV !== 'production') {
patchLightningElementPrototypeWithRestrictions(BaseLightningElementConstructor.prototype);
Expand Down
Loading

0 comments on commit 00b56c9

Please sign in to comment.