Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Add a useSignalEffect implementation that behaves like useEffect #226

Merged
merged 11 commits into from
May 4, 2023
23 changes: 23 additions & 0 deletions e2e/html/directives-effect.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!DOCTYPE html>
<html>
<head>
<title>Directives -- data-wp-effect</title>
<meta itemprop="wp-client-side-navigation" content="active" />
</head>

<body>
<div data-wp-show="state.isOpen">
<input data-testid="input" data-wp-effect="effects.writeToWindow" />
</div>

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

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

<script defer src="../../build/e2e/directive-effect.js"></script>
<script defer src="../../build/runtime.js"></script>
<script defer src="../../build/vendors.js"></script>
</body>
</html>
26 changes: 26 additions & 0 deletions e2e/js/directive-effect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { store } from '../../src/runtime/store';

store({
state: {
isOpen: true,
},
actions: {
toggle({ state }) {
state.isOpen = !state.isOpen;
},
},
effects: {
writeToWindow: () => {
window.effect = 'effect added';

return () => {
window.effect = 'effect removed';
};
},
changeFocus: ({ state }) => {
if (state.isOpen) {
document.querySelector("[data-testid='input']").focus();
}
},
},
});
26 changes: 26 additions & 0 deletions e2e/specs/directive-effect.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { test, expect } from '../tests';

test.describe('data-wp-effect', () => {
test.beforeEach(async ({ goToFile }) => {
await goToFile('directives-effect.html');
});

test('check that effect runs when it is added', async ({ page }) => {
const effect = await page.evaluate('window.effect');
expect(effect).toBe('effect added');
});

test('check that effect runs when it is removed', async ({ page }) => {
await page.getByTestId('toggle').click();
const effect = await page.evaluate('window.effect');
expect(effect).toBe('effect removed');
});

test('change focus after DOM changes', async ({ page }) => {
const el = page.getByTestId('input');
await expect(el).toBeFocused();
await page.getByTestId('toggle').click();
await page.getByTestId('toggle').click();
await expect(el).toBeFocused();
});
});
7 changes: 4 additions & 3 deletions src/runtime/directives.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useContext, useMemo, useEffect } from 'preact/hooks';
import { useSignalEffect } from '@preact/signals';
import { deepSignal, peek } from 'deepsignal';
import { useSignalEffect } from './utils';
import { directive } from './hooks';
import { prefetch, navigate, canDoClientSideNavigation } from './router';

Expand Down Expand Up @@ -48,7 +48,7 @@ export default () => {
const contextValue = useContext(context);
Object.values(effect).forEach((path) => {
useSignalEffect(() => {
evaluate(path, { context: contextValue });
return evaluate(path, { context: contextValue });
});
});
});
Expand All @@ -58,7 +58,7 @@ export default () => {
const contextValue = useContext(context);
Object.entries(on).forEach(([name, path]) => {
element.props[`on${name}`] = (event) => {
evaluate(path, { event, context: contextValue });
return evaluate(path, { event, context: contextValue });
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return is not needed; I'm pretty sure event listener callbacks always return undefined (see this). I'll change this, don't worry. 🙂

Copy link
Member

@luisherranz luisherranz May 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. So instead of returning false in the onclick attribute, you'd use event.stopPropagation() in the event listener. And Preact is using the listener, not the attribute, right?

};
});
});
Expand Down Expand Up @@ -185,6 +185,7 @@ export default () => {
context,
}) => {
const contextValue = useContext(context);

if (!evaluate(show, { context: contextValue }))
element.props.children = (
<template>{element.props.children}</template>
Expand Down
45 changes: 44 additions & 1 deletion src/runtime/utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,47 @@
// For wrapperless hydration of document.body.
import { useRef, useEffect } from 'preact/hooks';
import { effect } from '@preact/signals';

function afterNextFrame(callback) {
const done = () => {
cancelAnimationFrame(raf);
setTimeout(callback);
};
const raf = requestAnimationFrame(done);
}

// Using the mangled properties:
// this.c: this._callback
// this.x: this._compute
// https://github.com/preactjs/signals/blob/main/mangle.json
function createFlusher(compute, notify) {
let flush;
const dispose = effect(function () {
flush = this.c.bind(this);
this.x = compute;
this.c = notify;
return compute();
});
return { flush, dispose };
}

// 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;

useEffect(() => {
const execute = () => callback.current();
const notify = () => afterNextFrame(eff.flush);
const eff = createFlusher(execute, notify);
return eff.dispose;
}, []);
}

// For wrapperless hydration.
// See https://gist.github.com/developit/f4c67a2ede71dc2fab7f357f39cff28c
export const createRootFragment = (parent, replaceNode) => {
replaceNode = [].concat(replaceNode);
Expand Down
1 change: 1 addition & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = [
'e2e/page-1': './e2e/page-1',
'e2e/page-2': './e2e/page-2',
'e2e/directive-bind': './e2e/js/directive-bind',
'e2e/directive-effect': './e2e/js/directive-effect',
},
output: {
filename: '[name].js',
Expand Down