Skip to content

Commit

Permalink
[wip] Combine useRef and useState hooks
Browse files Browse the repository at this point in the history 
Since the useState hook is only used to force a render, and we don't
use it to track any state, we can stash our mutable instance object
in there instead of using a separate useRef hook. Doesn't affect any
behavior, just saves a bit of hook memory.
  • Loading branch information
@acdlite
acdlite committed Sep 1, 2021
1 parent 6a9738d commit 9e9df92
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 62 deletions.
33 changes: 0 additions & 33 deletions packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
};
}

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('basic usage', () => {
const store = createExternalStore('Initial');

Expand Down Expand Up @@ -141,9 +138,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('Initial');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('switch to a different store', () => {
const storeA = createExternalStore(0);
const storeB = createExternalStore(0);
Expand Down Expand Up @@ -193,9 +187,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('1');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('selecting a specific value inside getSnapshot', () => {
const store = createExternalStore({a: 0, b: 0});

Expand Down Expand Up @@ -239,9 +230,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('A1B1');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test(
"compares to current state before bailing out, even when there's a " +
'mutation in between the sync and passive effects',
Expand Down Expand Up @@ -284,9 +272,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
},
);

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('mutating the store in between render and commit when getSnapshot has changed', () => {
const store = createExternalStore({a: 1, b: 1});

Expand Down Expand Up @@ -345,9 +330,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('B2');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('mutating the store in between render and commit when getSnapshot has _not_ changed', () => {
// Same as previous test, but `getSnapshot` does not change
const store = createExternalStore({a: 1, b: 1});
Expand Down Expand Up @@ -405,9 +387,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('A1');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test("does not bail out if the previous update hasn't finished yet", () => {
const store = createExternalStore(0);

Expand Down Expand Up @@ -443,9 +422,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('00');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('uses the latest getSnapshot, even if it changed in the same batch as a store update', () => {
const store = createExternalStore({a: 0, b: 0});

Expand Down Expand Up @@ -473,9 +449,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('2');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('handles errors thrown by getSnapshot or isEqual', () => {
class ErrorBoundary extends React.Component {
state = {error: null};
Expand Down Expand Up @@ -552,9 +525,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
});

describe('extra features implemented in user-space', () => {
// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('memoized selectors are only called once per update', () => {
const store = createExternalStore({a: 0, b: 0});

Expand Down Expand Up @@ -593,9 +563,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
expect(root).toMatchRenderedOutput('A1');
});

// 18 has an experimental flag to warn about reading refs. Will circumvent
// when built-in API is implemented.
// @gate !enableUseRefAccessWarning || !__DEV__
test('Using isEqual to bailout', () => {
const store = createExternalStore({a: 0, b: 0});

Expand Down
46 changes: 17 additions & 29 deletions packages/use-sync-external-store/src/useSyncExternalStore.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import is from 'shared/objectIs';
// dispatch for CommonJS interop named imports.
const {
useState,
useRef,
useEffect,
useLayoutEffect,
useDebugValue,
Expand Down Expand Up @@ -59,34 +58,27 @@ function useSyncExternalStore_shim<T>(
}
}

// Because updates are synchronous, we don't queue them. Instead we force a
// re-render whenever the subscribed state changes, and read the current state
// directly in render.
//
// We force a re-render by bumping a version number.
const [, setVersion] = useState(0);

// Read the current snapshot from the store on every render. Again, this
// breaks the rules of React, and only works here because of specific
// implementation details, most importantly that updates are
// always synchronous.
const value = getSnapshot();

// Because updates are synchronous, we don't queue them. Instead we force a
// re-render whenever the subscribed state changes by updating an some
// arbitrary useState hook. Then, during render, we call getSnapshot to read
// the current value.
//
// Because we don't actually use the state returned by the useState hook, we
// can save a bit of memory by storing other stuff in that slot.
//
// To implement the early bailout, we need to track some things on a mutable
// ref object. Initialize this object on the first render.
const refs = useRef(null);
let inst;
if (refs.current === null) {
inst = {
// This represents the currently rendered value and getSnapshot function.
// We update them with a ref whenever they change.
value,
getSnapshot,
};
refs.current = inst;
} else {
inst = refs.current;
}
// object. Usually, we would put that with a useRef hook, but we can stash
// that in our useState hook instead.
//
// To force a re-render, we call forceUpdate({inst}). That works because the
// new object always fails an equality check.
const [{inst}, forceUpdate] = useState({inst: {value, getSnapshot}});

// Track the latest getSnapshot function with a ref. This needs to be updated
// in the layout phase so that we can access it during the tearing check that
Expand All @@ -102,7 +94,7 @@ function useSyncExternalStore_shim<T>(
// effect may have mutated the store.
if (checkIfSnapshotChanged(inst)) {
// Force a re-render.
setVersion(bumpVersion);
forceUpdate({inst});
}
}, [subscribe, value, getSnapshot]);

Expand All @@ -111,7 +103,7 @@ function useSyncExternalStore_shim<T>(
// detected in the subscription handler.
if (checkIfSnapshotChanged(inst)) {
// Force a re-render.
setVersion(bumpVersion);
forceUpdate({inst});
}
const handleStoreChange = () => {
// TODO: Because there is no cross-renderer API for batching updates, it's
Expand All @@ -123,7 +115,7 @@ function useSyncExternalStore_shim<T>(
// read from the store.
if (checkIfSnapshotChanged(inst)) {
// Force a re-render.
setVersion(bumpVersion);
forceUpdate({inst});
}
};
// Subscribe to the store and return a clean-up function.
Expand All @@ -144,7 +136,3 @@ function checkIfSnapshotChanged(inst) {
return true;
}
}

function bumpVersion(v) {
return v + 1;
}

0 comments on commit 9e9df92

Please sign in to comment.