Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactivity API: add short-cirtuit to useSignalEffect #53358

Merged
merged 4 commits into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,19 @@

<div data-wp-effect="effects.changeFocus"></div>

<div
data-testid="short-circuit infinite loops"
data-wp-effect="effects.infiniteLoop"
data-wp-text="state.counter"
>
0
</div>

<button data-testid="toggle" data-wp-on--click="actions.toggle">
Update
</button>

<button data-testid="increment" data-wp-on--click="actions.increment">
Increment
</button>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
state: {
isOpen: true,
isElementInTheDOM: false,
counter: 0,
},
selectors: {
elementInTheDOM: ( { state } ) =>
Expand All @@ -32,6 +33,9 @@
toggle( { state } ) {
state.isOpen = ! state.isOpen;
},
increment( { state } ) {
state.counter = state.counter + 1;
},
},
effects: {
elementAddedToTheDOM: ( { state } ) => {
Expand All @@ -46,6 +50,9 @@
document.querySelector( "[data-testid='input']" ).focus();
}
},
infiniteLoop: ({ state }) => {
state.counter = state.counter + 1;
}
},
} );

Expand Down
1 change: 1 addition & 0 deletions packages/interactivity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug Fix

- Add support for underscores and leading dashes in the suffix part of the directive. ([#53337](https://github.com/WordPress/gutenberg/pull/53337))
- Add an asynchronous short circuit to `useSignalEffect` to avoid infinite loops. ([#53358](https://github.com/WordPress/gutenberg/pull/53358))

### Enhancements

Expand Down
47 changes: 28 additions & 19 deletions packages/interactivity/src/utils.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
/**
* External dependencies
*/
import { useRef, useEffect } from 'preact/hooks';
import { useEffect } from 'preact/hooks';
import { effect } from '@preact/signals';

function afterNextFrame( callback ) {
const done = () => {
window.cancelAnimationFrame( raf );
setTimeout( callback );
};
const raf = window.requestAnimationFrame( done );
}
const afterNextFrame = ( callback ) => {
return new Promise( ( resolve ) => {
const done = () => {
clearTimeout( timeout );
window.cancelAnimationFrame( raf );
setTimeout( () => {
callback();
resolve();
} );
};
const timeout = setTimeout( done, 100 );
const raf = window.requestAnimationFrame( done );
DAreRodz marked this conversation as resolved.
Show resolved Hide resolved
} );
};

// Using the mangled properties:
// this.c: this._callback
Expand All @@ -28,18 +35,20 @@ function createFlusher( compute, notify ) {
}

// Version of `useSignalEffect` with a `useEffect`-like execution. This hook
// implementation comes from this PR:
// https://github.com/preactjs/signals/pull/290.
//
// We need to include it here in this repo until the mentioned PR is merged.
export function useSignalEffect( cb ) {
const callback = useRef( cb );
callback.current = cb;

// implementation comes from this PR, but we added short-cirtuiting to avoid
// infinite loops: https://github.com/preactjs/signals/pull/290
export function useSignalEffect( callback ) {
useEffect( () => {
const execute = () => callback.current();
const notify = () => afterNextFrame( eff.flush );
const eff = createFlusher( execute, notify );
let eff = null;
let isExecuting = false;
const notify = async () => {
if ( eff && ! isExecuting ) {
isExecuting = true;
await afterNextFrame( eff.flush );
isExecuting = false;
}
};
eff = createFlusher( callback, notify );
return eff.dispose;
}, [] );
}
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/specs/interactivity/directive-effect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,11 @@ test.describe( 'data-wp-effect', () => {
await page.getByTestId( 'toggle' ).click();
await expect( el ).toBeFocused();
} );

test( 'short-circuit infinite loops', async ( { page } ) => {
const el = page.getByTestId( 'short-circuit infinite loops' );
await expect( el ).toContainText( '1' );
await page.getByTestId( 'increment' ).click();
await expect( el ).toContainText( '3' );
} );
} );