|
| 1 | +import { createElement, setFeatureFlagForTest } from 'lwc'; |
| 2 | + |
| 3 | +import Test from 'x/test'; |
| 4 | + |
| 5 | +// A bug (W-19830319) has shown that if child component properties are incorrectly marked for observation by the lifecycle, they can trigger the parent component to re-render |
| 6 | +// and this will cause undesired effects. This bug manifested due to context connection/disconnection erroneously accessing properties, but it could theoretically occur in any instance where |
| 7 | +// the "live" component property is accessed instead of via vm.cmpFields / vm.cmpProps. |
| 8 | +describe('Unobserved properties should trigger additional re-renders', () => { |
| 9 | + it('when signals is enabled and legacy context connection is enabled', async () => { |
| 10 | + // This is on by default, but enabled here to confirm the case |
| 11 | + setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true); |
| 12 | + setFeatureFlagForTest('ENABLE_LEGACY_CONTEXT_CONNECTION', true); |
| 13 | + |
| 14 | + const elm = createElement('x-test', { is: Test }); |
| 15 | + document.body.appendChild(elm); |
| 16 | + // Specific sequence (it is rather nuanced and requires 1. parent to switch templates and 2. child to mutate a property in a setTimeout function or similar): |
| 17 | + // 1. The template change results in the parent component being marked as dirty. |
| 18 | + // 1a. Marking the parent as dirty sets the currentReactiveObserver to the parent, here: https://github.com/salesforce/lwc/blob/master/packages/%40lwc/engine-core/src/libs/mutation-tracker/index.ts#L83 |
| 19 | + // 2. The new template doesn't contain the child so disconnectContext is called on the child component. The BUG: If the child properties are incorrectly observed then riggering disconnectContext marks all child properties |
| 20 | + // for observation using the currentReactiveObserver of the parent set in 1a. here: https://github.com/salesforce/lwc/blob/master/packages/%40lwc/engine-core/src/libs/mutation-tracker/index.ts#L60 |
| 21 | + // 3. Next, a delayed property mutation inside the child component's renderedCallback occurs and this delayed (post disconnection) mutation triggers valueMutated, where all the parent properties are registered listeners. |
| 22 | + // That happens here: https://github.com/salesforce/lwc/blob/master/packages/%40lwc/engine-core/src/libs/mutation-tracker/index.ts#L41 |
| 23 | + // 3b. This causes the parent component to re-render. |
| 24 | + elm.switchToEmptyTemplate(); |
| 25 | + |
| 26 | + // Before the bugfix (W-19830319/PR-5536), expecting 2 renders when signals was enabled would fail. |
| 27 | + // An erroneous 3rd render would occur when signals was enabled due to the behavior described above. |
| 28 | + setTimeout(() => { |
| 29 | + expect(elm.renderCount).toBe(3); // because ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION = true this will render 3 times (incorrect) |
| 30 | + }); |
| 31 | + }); |
| 32 | + |
| 33 | + it('when signals is enabled, legacy context connection is true and detached rehydration is enabled', async () => { |
| 34 | + setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true); |
| 35 | + setFeatureFlagForTest('ENABLE_LEGACY_CONTEXT_CONNECTION', true); |
| 36 | + setFeatureFlagForTest('DISABLE_DETACHED_REHYDRATION', true); |
| 37 | + |
| 38 | + const elm = createElement('x-test', { is: Test }); |
| 39 | + document.body.appendChild(elm); |
| 40 | + elm.switchToEmptyTemplate(); |
| 41 | + |
| 42 | + // Enabling detached rehydration does not change the behavior (bug still exists and the third render occurs) |
| 43 | + setTimeout(() => { |
| 44 | + expect(elm.renderCount).toBe(3); |
| 45 | + }); |
| 46 | + }); |
| 47 | + |
| 48 | + afterAll(() => { |
| 49 | + setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true); |
| 50 | + setFeatureFlagForTest('ENABLE_LEGACY_CONTEXT_CONNECTION', false); |
| 51 | + setFeatureFlagForTest('DISABLE_DETACHED_REHYDRATION', false); |
| 52 | + }); |
| 53 | +}); |
| 54 | + |
| 55 | +describe('Unobserved properties should NOT trigger additional re-renders', () => { |
| 56 | + it('when signals is enabled, legacy context connection is disabled and detached rehydration is enabled', async () => { |
| 57 | + setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true); |
| 58 | + setFeatureFlagForTest('ENABLE_LEGACY_CONTEXT_CONNECTION', false); |
| 59 | + setFeatureFlagForTest('DISABLE_DETACHED_REHYDRATION', true); |
| 60 | + |
| 61 | + const elm = createElement('x-test', { is: Test }); |
| 62 | + document.body.appendChild(elm); |
| 63 | + elm.switchToEmptyTemplate(); |
| 64 | + |
| 65 | + // Enabling detached rehydration does not change the behavior |
| 66 | + setTimeout(() => { |
| 67 | + expect(elm.renderCount).toBe(2); |
| 68 | + }); |
| 69 | + }); |
| 70 | + |
| 71 | + it('when signals is enabled and legacy context connection is disabled', async () => { |
| 72 | + setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true); |
| 73 | + setFeatureFlagForTest('ENABLE_LEGACY_CONTEXT_CONNECTION', false); |
| 74 | + |
| 75 | + const elm = createElement('x-test', { is: Test }); |
| 76 | + document.body.appendChild(elm); |
| 77 | + elm.switchToEmptyTemplate(); |
| 78 | + |
| 79 | + // CORRECT: This component should render twice as the corrected context connection is in use. |
| 80 | + setTimeout(() => { |
| 81 | + expect(elm.renderCount).toBe(2); |
| 82 | + }); |
| 83 | + }); |
| 84 | + |
| 85 | + it('when signals is disabled', async () => { |
| 86 | + setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', false); |
| 87 | + |
| 88 | + const elm = createElement('x-test', { is: Test }); |
| 89 | + document.body.appendChild(elm); |
| 90 | + elm.switchToEmptyTemplate(); |
| 91 | + |
| 92 | + // CORRECT: This component should render twice as signals is disabled and there should be no observation bugs elsewhere in the lifecycle. |
| 93 | + setTimeout(() => { |
| 94 | + expect(elm.renderCount).toBe(2); |
| 95 | + }); |
| 96 | + }); |
| 97 | + |
| 98 | + afterAll(() => { |
| 99 | + setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true); |
| 100 | + setFeatureFlagForTest('DISABLE_DETACHED_REHYDRATION', false); |
| 101 | + }); |
| 102 | +}); |
0 commit comments