Skip to content

Commit

Permalink
fix(engine): live bindings for value and checked properties 3nd attem…
Browse files Browse the repository at this point in the history
…pt (#340)
  • Loading branch information
caridy authored and Trevor committed May 24, 2018
1 parent 1e4c7f1 commit ef4acff
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 26 deletions.
5 changes: 3 additions & 2 deletions packages/lwc-engine/src/framework/__tests__/component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe('component', function() {
expect(context).toBe(elm[ViewModelReflection].component);
});

it('should fail to execute setter function when used directly from DOM', function() {
it('should call setter function when used directly from DOM', function() {
class MyChild extends Element {
value = 'pancakes';
get breakfast() {
Expand All @@ -188,12 +188,13 @@ describe('component', function() {
}
run() {
this.template.querySelector('x-child').breakfast = 'eggs';
return this.template.querySelector('x-child').breakfast;
}
}
MyComponent.publicMethods = ['run'];
const elm = createElement('x-foo', { is: MyComponent });
document.body.appendChild(elm);
expect(() => elm.run()).toThrow();
expect(elm.run()).toBe('eggs');
});

it('should execute setter function with correct context when component is root', function() {
Expand Down
50 changes: 26 additions & 24 deletions packages/lwc-engine/src/framework/decorators/api.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import assert from "../assert";
import { isRendering, vmBeingRendered, isBeingConstructed } from "../invoker";
import { isObject, isNull, isTrue, hasOwnProperty } from "../language";
import { isObject, isNull, isTrue, hasOwnProperty, toString } from "../language";
import { observeMutation, notifyMutation } from "../watcher";
import { Component, ComponentConstructor } from "../component";
import { VM, getCustomElementVM } from "../vm";
Expand Down Expand Up @@ -81,20 +81,21 @@ export function createPublicPropertyDescriptor(proto: ComponentConstructor, key:
}
}
}
if (vmBeingUpdated === vm) {
// not need to wrap or check the value since that is happening somewhere else
vmBeingUpdated = null; // releasing the lock
vm.cmpProps[key] = reactiveMembrane.getReadOnlyProxy(newValue);

// avoid notification of observability while constructing the instance
if (vm.idx > 0) {
// perf optimization to skip this step if not in the DOM
notifyMutation(this, key);
if (process.env.NODE_ENV !== 'production') {
if (vmBeingUpdated !== vm) {
// logic for setting new properties of the element directly from the DOM
// is only recommended for root elements created via createElement()
assert.logWarning(`If property ${key} decorated with @api in ${vm} is used in the template, the value ${toString(newValue)} set manually may be overridden by the template, consider binding the property only in the template.`);
}
} else if (process.env.NODE_ENV !== 'production') {
// logic for setting new properties of the element directly from the DOM
// will only be allowed for root elements created via createElement()
assert.logError(`Invalid attempt to set property ${key} from ${vm} to ${newValue}. This property was decorated with @api, and can only be changed via the template.`);
}
vmBeingUpdated = null; // releasing the lock
// not need to wrap or check the value since that is happening somewhere else
vm.cmpProps[key] = reactiveMembrane.getReadOnlyProxy(newValue);

// avoid notification of observability while constructing the instance
if (vm.idx > 0) {
// perf optimization to skip this step if not in the DOM
notifyMutation(this, key);
}
},
enumerable: isUndefined(descriptor) ? true : descriptor.enumerable,
Expand Down Expand Up @@ -134,18 +135,19 @@ export function createPublicAccessorDescriptor(Ctor: ComponentConstructor, key:
}
}
}
if (vmBeingUpdated === vm) {
// not need to wrap or check the value since that is happening somewhere else
vmBeingUpdated = null; // releasing the lock
if (set) {
set.call(this, reactiveMembrane.getReadOnlyProxy(newValue));
} else if (process.env.NODE_ENV !== 'production') {
assert.fail(`Invalid attempt to set a new value for property ${key} of ${vm} that does not has a setter decorated with @api.`);
if (process.env.NODE_ENV !== 'production') {
if (vmBeingUpdated !== vm) {
// logic for setting new properties of the element directly from the DOM
// is only recommended for root elements created via createElement()
assert.logWarning(`If property ${key} decorated with @api in ${vm} is used in the template, the value ${toString(newValue)} set manually may be overridden by the template, consider binding the property only in the template.`);
}
}
vmBeingUpdated = null; // releasing the lock
// not need to wrap or check the value since that is happening somewhere else
if (set) {
set.call(this, reactiveMembrane.getReadOnlyProxy(newValue));
} else if (process.env.NODE_ENV !== 'production') {
// logic for setting new properties of the element directly from the DOM
// will only be allowed for root elements created via createElement()
assert.fail(`Invalid attempt to set property ${key} from ${vm} to ${newValue}. This property was decorated with @api, and can only be changed via the template.`);
assert.fail(`Invalid attempt to set a new value for property ${key} of ${vm} that does not has a setter decorated with @api.`);
}
},
enumerable,
Expand Down

0 comments on commit ef4acff

Please sign in to comment.