Skip to content

Commit

Permalink
fix: initial state was reset in single subscription hooks in strict m…
Browse files Browse the repository at this point in the history
…ode in dev

With strict mode in development, each component is mounted, unmounted and remounted.
Previously, `initialState` was set to `undefined` after the first mount because a change in the passed reference was assumed.
  • Loading branch information
andipaetzold committed Aug 15, 2024
1 parent 83d1bc6 commit d0db2af
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 34 deletions.
46 changes: 31 additions & 15 deletions src/internal/useListen.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { act, renderHook } from "@testing-library/react";
import { act, configure, renderHook } from "@testing-library/react";
import { newSymbol } from "../__testfixtures__/index.js";
import { useListen } from "./useListen.js";
import { LoadingState } from "./useLoadingValue.js";
import { it, expect, beforeEach, describe, vi } from "vitest";
import { it, expect, beforeEach, describe, vi, afterEach } from "vitest";

const result1 = newSymbol("Result 1");
const result2 = newSymbol("Result 2");
Expand All @@ -25,21 +25,37 @@ beforeEach(() => {
onChange.mockReturnValue(onChangeUnsubscribe);
});

describe("initial state", () => {
// reference, initialState, expectedValue, expectedLoading
it.each([
[undefined, result1, undefined, false],
[undefined, undefined, undefined, false],
[undefined, LoadingState, undefined, false],
[refA1, result1, result1, false],
[refA1, undefined, undefined, false],
[refA1, LoadingState, undefined, true],
])("reference=%s initialState=%s", (reference, initialState, expectedValue, expectedLoading) => {
const { result } = renderHook(() => useListen(reference, onChange, isEqual, initialState));
expect(result.current).toStrictEqual([expectedValue, expectedLoading, undefined]);
});
afterEach(() => {
configure({ reactStrictMode: false });
});

describe.each([{ reactStrictMode: true }, { reactStrictMode: false }])(
`strictMode=$reactStrictMode`,
({ reactStrictMode }) => {
beforeEach(() => {
configure({ reactStrictMode });
});

describe("initial state", () => {
it.each`
reference | initialState | expectedValue | expectedLoading
${undefined} | ${result1} | ${undefined} | ${false}
${undefined} | ${undefined} | ${undefined} | ${false}
${undefined} | ${LoadingState} | ${undefined} | ${false}
${refA1} | ${result1} | ${result1} | ${false}
${refA1} | ${undefined} | ${undefined} | ${false}
${refA1} | ${LoadingState} | ${undefined} | ${true}
`(
"reference=$reference initialState=$initialState",
({ reference, initialState, expectedValue, expectedLoading }) => {
const { result } = renderHook(() => useListen(reference, onChange, isEqual, initialState));
expect(result.current).toStrictEqual([expectedValue, expectedLoading, undefined]);
},
);
});
},
);

describe("when changing ref", () => {
it("should not resubscribe for equal ref", async () => {
// first ref
Expand Down
38 changes: 19 additions & 19 deletions src/internal/useListen.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { useEffect, useMemo, useRef } from "react";
import { useEffect, useMemo } from "react";
import { ValueHookResult } from "../common/index.js";
import { useLoadingValue, LoadingState } from "./useLoadingValue.js";
import { useStableValue } from "./useStableValue.js";
import { usePrevious } from "./usePrevious.js";

/**
* @internal
Expand All @@ -26,30 +27,29 @@ export function useListen<Value, Error, Reference>(
);

const stableRef = useStableValue(reference ?? undefined, isEqual);
const firstRender = useRef<boolean>(true);
const previousRef = usePrevious(stableRef);

// set state when ref changes
useEffect(() => {
if (stableRef === previousRef) {
return;
}

if (stableRef === undefined) {
// value doesn't change on first render with undefined ref
if (firstRender.current) {
firstRender.current = false;
} else {
setValue();
}
setValue();
} else {
// do not set loading state on first render
// otherwise, the defaultValue gets overwritten
if (firstRender.current) {
firstRender.current = false;
} else {
setLoading();
}
setLoading();
}
}, [previousRef, setLoading, setValue, stableRef]);

const unsubscribe = onChange(stableRef, setValue, setError);
return () => unsubscribe();
// subscribe to changes
useEffect(() => {
if (stableRef === undefined) {
return;
}
return undefined;
}, [stableRef, onChange, setError, setLoading, setValue]);
const unsubscribe = onChange(stableRef, setValue, setError);
return () => unsubscribe();
}, [onChange, setError, setValue, stableRef]);

return useMemo(() => [value, loading, error], [value, loading, error]);
}
9 changes: 9 additions & 0 deletions src/internal/usePrevious.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { useEffect, useRef } from "react";

export function usePrevious<T>(value: T) {
const ref = useRef<T>(value);
useEffect(() => {
ref.current = value;
});
return ref.current;
}

0 comments on commit d0db2af

Please sign in to comment.