Skip to content

Commit

Permalink
fix(signals): avoid throwing when in check throws
Browse files Browse the repository at this point in the history
Fixes #5104
  • Loading branch information
nolanlawson committed Jan 8, 2025
1 parent 6bf7070 commit a310e55
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 4 deletions.
9 changes: 5 additions & 4 deletions packages/@lwc/engine-core/src/framework/mutation-tracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { isFunction, isNull, isObject, isTrustedSignal } from '@lwc/shared';
import { ReactiveObserver, valueMutated, valueObserved } from '../libs/mutation-tracker';
import { subscribeToSignal } from '../libs/signal-tracker';
import { safeHasProp } from './utils';
import type { Signal } from '@lwc/signals';
import type { JobFunction, CallbackFunction } from '../libs/mutation-tracker';
import type { VM } from './vm';
Expand Down Expand Up @@ -34,16 +35,16 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = {
}

// The portion of reactivity that's exposed to signals is to subscribe a callback to re-render the VM (templates).
// We check check the following to ensure re-render is subscribed at the correct time.
// We check the following to ensure re-render is subscribed at the correct time.
// 1. The template is currently being rendered (there is a template reactive observer)
// 2. There was a call to a getter to access the signal (happens during vnode generation)
if (
lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS &&
isObject(target) &&
!isNull(target) &&
'value' in target &&
'subscribe' in target &&
isFunction(target.subscribe) &&
safeHasProp(target, 'value') &&
safeHasProp(target, 'subscribe') &&
isFunction((target as any).subscribe) &&
isTrustedSignal(target) &&
// Only subscribe if a template is being rendered by the engine
tro.isObserving()
Expand Down
10 changes: 10 additions & 0 deletions packages/@lwc/engine-core/src/framework/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,13 @@ export function shouldBeFormAssociated(Ctor: LightningElementConstructor) {

return ctorFormAssociated && apiFeatureEnabled;
}

// check if a property is in an object, and if the object throws an error merely because we are
// checking if the property exists, return false
export function safeHasProp(obj: any, prop: string) {
try {
return prop in obj;
} catch (_err) {
return false;
}
}
10 changes: 10 additions & 0 deletions packages/@lwc/integration-karma/test/signal/protocol/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Parent from 'x/parent';
import Child from 'x/child';
import DuplicateSignalOnTemplate from 'x/duplicateSignalOnTemplate';
import List from 'x/list';
import Throws from 'x/throws';

// Note for testing purposes the signal implementation uses LWC module resolution to simplify things.
// In production the signal will come from a 3rd party library.
Expand Down Expand Up @@ -212,6 +213,15 @@ describe('signal protocol', () => {

expect(subscribe).not.toHaveBeenCalled();
});

fit('does not throw an error for objects that throw upon "in" checks', async () => {
const elm = createElement('x-throws', { is: Throws });
document.body.appendChild(elm);

await Promise.resolve();

expect(elm.shadowRoot.querySelector('h1').textContent).toBe('hello');
});
});

describe('ENABLE_EXPERIMENTAL_SIGNALS not set', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
<h1>hello</h1>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { LightningElement } from 'lwc';

export default class extends LightningElement {
foo;

constructor() {
super();

this.foo = new Proxy(
{},
{
has() {
throw new Error("oh no you don't!");
},
}
);
}

renderedCallback() {
// access `this.foo` to trigger mutation-tracker.ts
this.bar = this.foo;
}
}

0 comments on commit a310e55

Please sign in to comment.