Skip to content

Commit

Permalink
Fix rendering signals as text when using react-transform (#439)
Browse files Browse the repository at this point in the history
* On exported useSignals, only run logic if auto is not installed
* Remove top-level requirement from react-transform
* Add return types to react runtime exports
  • Loading branch information
andrewiggins authored Nov 16, 2023
1 parent beafe69 commit fb6b050
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/fresh-panthers-explain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@preact/signals-react-transform": patch
---

Remove top-level requirement from react-transform
5 changes: 5 additions & 0 deletions .changeset/lovely-moons-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@preact/signals-react": patch
---

Fix rendering signals as text when using react-transform
8 changes: 2 additions & 6 deletions packages/react-transform/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,12 @@ function isOptedOutOfSignalTracking(path: NodePath | null): boolean {
function isComponentFunction(path: NodePath<FunctionLike>): boolean {
return (
fnNameStartsWithCapital(path) && // Function name indicates it's a component
getData(path.scope, containsJSX) === true && // Function contains JSX
path.scope.parent === path.scope.getProgramParent() // Function is top-level
getData(path.scope, containsJSX) === true // Function contains JSX
);
}

function isCustomHook(path: NodePath<FunctionLike>): boolean {
return (
fnNameStartsWithUse(path) && // Function name indicates it's a hook
path.scope.parent === path.scope.getProgramParent()
); // Function is top-level
return fnNameStartsWithUse(path); // Function name indicates it's a hook
}

function shouldTransform(
Expand Down
8 changes: 6 additions & 2 deletions packages/react/runtime/src/auto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
import React from "react";
import jsxRuntime from "react/jsx-runtime";
import jsxRuntimeDev from "react/jsx-dev-runtime";
import { EffectStore, useSignals, wrapJsx } from "./index";
import { EffectStore, wrapJsx, _useSignalsImplementation } from "./index";

export interface ReactDispatcher {
useRef: typeof React.useRef;
Expand Down Expand Up @@ -146,11 +146,15 @@ const dispatcherMachinePROD = createMachine({
```
*/

export let isAutoSignalTrackingInstalled = false;

let store: EffectStore | null = null;
let lock = false;
let currentDispatcher: ReactDispatcher | null = null;

function installCurrentDispatcherHook() {
isAutoSignalTrackingInstalled = true;

Object.defineProperty(ReactInternals.ReactCurrentDispatcher, "current", {
get() {
return currentDispatcher;
Expand All @@ -171,7 +175,7 @@ function installCurrentDispatcherHook() {
isEnteringComponentRender(currentDispatcherType, nextDispatcherType)
) {
lock = true;
store = useSignals();
store = _useSignalsImplementation();
lock = false;
} else if (
isExitingComponentRender(currentDispatcherType, nextDispatcherType)
Expand Down
53 changes: 47 additions & 6 deletions packages/react/runtime/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import { signal, computed, effect, Signal } from "@preact/signals-core";
import {
signal,
computed,
effect,
Signal,
ReadonlySignal,
} from "@preact/signals-core";
import { useRef, useMemo, useEffect } from "react";
import { useSyncExternalStore } from "use-sync-external-store/shim/index.js";
import { isAutoSignalTrackingInstalled } from "./auto";

export { installAutoSignalTracking } from "./auto";

const Empty = [] as const;
const ReactElemType = Symbol.for("react.element"); // https://github.com/facebook/react/blob/346c7d4c43a0717302d446da9e7423a8e28d8996/packages/shared/ReactSymbols.js#L15
const noop = () => {};

export function wrapJsx<T>(jsx: T): T {
if (typeof jsx !== "function") return jsx;
Expand Down Expand Up @@ -113,14 +121,37 @@ function createEffectStore(): EffectStore {
};
}

function createEmptyEffectStore(): EffectStore {
return {
effect: {
_sources: undefined,
_callback() {},
_start() {
return noop;
},
_dispose() {},
},
subscribe() {
return noop;
},
getSnapshot() {
return 0;
},
f() {},
[symDispose]() {},
};
}

const emptyEffectStore = createEmptyEffectStore();

let finalCleanup: Promise<void> | undefined;
const _queueMicroTask = Promise.prototype.then.bind(Promise.resolve());

/**
* Custom hook to create the effect to track signals used during render and
* subscribe to changes to rerender the component when the signals change.
*/
export function useSignals(): EffectStore {
export function _useSignalsImplementation(): EffectStore {
clearCurrentStore();
if (!finalCleanup) {
finalCleanup = _queueMicroTask(() => {
Expand All @@ -145,7 +176,12 @@ export function useSignals(): EffectStore {
* A wrapper component that renders a Signal's value directly as a Text node or JSX.
*/
function SignalValue({ data }: { data: Signal }) {
return data.value;
const store = useSignals();
try {
return data.value;
} finally {
store.f();
}
}

// Decorate Signals so React renders them as <SignalValue> components.
Expand All @@ -161,17 +197,22 @@ Object.defineProperties(Signal.prototype, {
ref: { configurable: true, value: null },
});

export function useSignal<T>(value: T) {
export function useSignals(): EffectStore {
if (isAutoSignalTrackingInstalled) return emptyEffectStore;
return _useSignalsImplementation();
}

export function useSignal<T>(value: T): Signal<T> {
return useMemo(() => signal<T>(value), Empty);
}

export function useComputed<T>(compute: () => T) {
export function useComputed<T>(compute: () => T): ReadonlySignal<T> {
const $compute = useRef(compute);
$compute.current = compute;
return useMemo(() => computed<T>(() => $compute.current()), Empty);
}

export function useSignalEffect(cb: () => void | (() => void)) {
export function useSignalEffect(cb: () => void | (() => void)): void {
const callback = useRef(cb);
callback.current = cb;

Expand Down

0 comments on commit fb6b050

Please sign in to comment.