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

Fix React.lazy and React.forwardRef component updates tracking. #427

Open
wants to merge 6 commits into
base: react-1.2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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
20 changes: 20 additions & 0 deletions .changeset/shiny-lizards-allow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
"@preact/signals-react": patch
---

Added reactivity for components wrapped with `React.forwardRef` and `React.lazy`.
So since this moment, this code will work as expected:
```tsx
const sig = signal(0)
setInterval(() => sig.value++, 1000)

const Lazy = React.lazy(() => Promise.resolve({ default: () => <div>{sig.value + 1}</div> }))
const Forwarded = React.forwardRef(() => <div>{sig.value + 1}</div>)

export const App = () => (
<Suspense>
<Lazy />
<Forwarded />
</Suspense>
)
```
39 changes: 39 additions & 0 deletions packages/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
useEffect,
Component,
type FunctionComponent,
ForwardRefExoticComponent,
} from "react";
import React from "react";
import jsxRuntime from "react/jsx-runtime";
Expand All @@ -24,10 +25,16 @@ export { signal, computed, batch, effect, Signal, type ReadonlySignal };
const Empty = [] as const;
const ReactElemType = Symbol.for("react.element"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L15
const ReactMemoType = Symbol.for("react.memo"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L30
const ReactLazyType = Symbol.for("react.lazy"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L31
const ReactForwardRefType: symbol = Symbol.for("react.forward_ref"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L25
const ProxyInstance = new WeakMap<
FunctionComponent<any>,
FunctionComponent<any>
>();
const ProxyForwardRef = new WeakMap<
ForwardRefExoticComponent<any>,
ForwardRefExoticComponent<any>
>();
const SupportsProxy = typeof Proxy === "function";

const ProxyHandlers = {
Expand Down Expand Up @@ -67,6 +74,22 @@ const ProxyHandlers = {
function ProxyFunctionalComponent(Component: FunctionComponent<any>) {
return ProxyInstance.get(Component) || WrapWithProxy(Component);
}
function ProxyForwardRefComponent(Component: ForwardRefExoticComponent<any>) {
const current = ProxyForwardRef.get(Component);
if (current) {
return current;
}
const WrappedComponent: ForwardRefExoticComponent<any> = {
$$typeof: ReactForwardRefType,
// @ts-expect-error React lies about ForwardRefExtoricComponent type
render: ProxyFunctionalComponent(Component.render),
};

ProxyForwardRef.set(Component, WrappedComponent);
ProxyForwardRef.set(WrappedComponent, WrappedComponent);

return WrappedComponent;
}
function WrapWithProxy(Component: FunctionComponent<any>) {
if (SupportsProxy) {
const ProxyComponent = new Proxy(Component, ProxyHandlers);
Expand Down Expand Up @@ -166,6 +189,22 @@ function WrapJsx<T>(jsx: T): T {
return jsx.call(jsx, type, props, ...rest);
}

if (type && typeof type === "object" && type.$$typeof === ReactLazyType) {
return jsx.call(
jsx,
ProxyFunctionalComponent(type._init(type._payload)),
props,
...rest
);
}
if (
type &&
typeof type === "object" &&
type.$$typeof === ReactForwardRefType
) {
return jsx.call(jsx, ProxyForwardRefComponent(type), props, ...rest);
}

if (typeof type === "string" && props) {
for (let i in props) {
let v = props[i];
Expand Down
104 changes: 103 additions & 1 deletion packages/react/test/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,16 @@
globalThis.IS_REACT_ACT_ENVIRONMENT = true;

import { signal, useComputed, useSignalEffect } from "@preact/signals-react";
import { createElement, useMemo, memo, StrictMode, createRef } from "react";
import {
createElement,
useMemo,
memo,
StrictMode,
createRef,
forwardRef,
lazy,
Suspense,
} from "react";
import { createRoot, Root } from "react-dom/client";
import { renderToStaticMarkup } from "react-dom/server";
import { act } from "react-dom/test-utils";
Expand Down Expand Up @@ -160,6 +169,79 @@ describe("@preact/signals-react", () => {
expect(scratch.textContent).to.equal("bar");
});

it("should update components wrapped with memo via signals", async () => {
const sig = signal("foo");

const Inner = memo(() => {
const value = sig.value;
return <p>{value}</p>;
});

function App() {
return <Inner />;
}

render(<App />);
expect(scratch.textContent).to.equal("foo");

act(() => {
sig.value = "bar";
});
expect(scratch.textContent).to.equal("bar");
});
it("should update components wrapped with lazy via signals", async () => {
const sig = signal("foo");

const _Inner = () => {
const value = sig.value;
return <p>{value}</p>;
};
let pr: undefined | Promise<unknown>;
const Inner = lazy(() => (pr = Promise.resolve({ default: _Inner })));

function App() {
return (
<Suspense>
<Inner />
</Suspense>
);
}

render(<App />);
expect(pr).instanceOf(Promise);
expect(scratch.textContent).not.to.equal("foo");
await act(async () => {
await pr;
});
expect(scratch.textContent).to.equal("foo");

act(() => {
sig.value = "bar";
});
expect(scratch.textContent).to.equal("bar");
});

it("should update components wrapped with forwardRef via signals", async () => {
const sig = signal("foo");

const Inner = forwardRef(() => {
const value = sig.value;
return <p>{value}</p>;
});

function App() {
return <Inner />;
}

render(<App />);
expect(scratch.textContent).to.equal("foo");

act(() => {
sig.value = "bar";
});
expect(scratch.textContent).to.equal("bar");
});

it("should consistently rerender in strict mode", async () => {
const sig = signal<string>(null!);

Expand Down Expand Up @@ -200,6 +282,26 @@ describe("@preact/signals-react", () => {
expect(scratch.textContent).to.equal(value);
}
});
it("should consistently rerender in strict mode (with forwardRef)", async () => {
const sig = signal<string>(null!);

const Test = forwardRef(() => <p>{sig.value}</p>);
const App = () => (
<StrictMode>
<Test />
</StrictMode>
);

for (let i = 0; i < 3; i++) {
const value = `${i}`;

act(() => {
sig.value = value;
render(<App />);
});
expect(scratch.textContent).to.equal(value);
}
});
it("should render static markup of a component", async () => {
const count = signal(0);

Expand Down
Loading