Skip to content

Commit

Permalink
Refactor/remove unused dispatcher code and implement useCallback (#42)
Browse files Browse the repository at this point in the history
* Remove unnecessary isReRender flag

We can derive this from workInProgressHook.queue and
in useReducer we keep an update queue anyway.

* Remove unnecessary guard in useMemo

* Implement guaranteed useCallback hook

Previously useCallback was a noop, however
this leads to confusing situations where people
rely on useCallback being a stable reference.

* Replace useCallback with useMemo impl
  • Loading branch information
kitten authored Feb 19, 2020
1 parent 3eee22b commit 4e557aa
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 37 deletions.
7 changes: 0 additions & 7 deletions src/internals/__tests__/dispatcher.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ describe('useEffect', () => {
})
})

describe('useCallback', () => {
it('is an identity function', () => {
const fn = () => {}
expect(Dispatcher.useCallback(fn)).toBe(fn)
})
})

describe('useTransition', () => {
it('returns noop and false', () => {
const result = Dispatcher.useTransition()
Expand Down
43 changes: 13 additions & 30 deletions src/internals/dispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ export const getCurrentIdentity = (): Identity => {

let firstWorkInProgressHook: Hook | null = null
let workInProgressHook: Hook | null = null
// Whether the work-in-progress hook is a re-rendered hook
let isReRender: boolean = false
// Whether an update was scheduled during the currently executing render pass.
let didScheduleRenderPhaseUpdate: boolean = false
// Lazily created map of render-phase updates
Expand All @@ -62,14 +60,10 @@ function areHookInputsEqual(
) {
// NOTE: The warnings that are used in ReactPartialRendererHooks are obsolete
// in a prepass, since these issues will be caught by a subsequent renderer anyway
if (prevDeps === null) {
return false
}
if (prevDeps === null) return false

for (let i = 0; i < prevDeps.length && i < nextDeps.length; i++) {
if (!is(nextDeps[i], prevDeps[i])) {
return false
}
if (!is(nextDeps[i], prevDeps[i])) return false
}

return true
Expand All @@ -87,25 +81,20 @@ function createWorkInProgressHook(): Hook {
if (workInProgressHook === null) {
// This is the first hook in the list
if (firstWorkInProgressHook === null) {
isReRender = false
firstWorkInProgressHook = workInProgressHook = createHook()
return (firstWorkInProgressHook = workInProgressHook = createHook())
} else {
// There's already a work-in-progress. Reuse it.
isReRender = true
workInProgressHook = firstWorkInProgressHook
return (workInProgressHook = firstWorkInProgressHook)
}
} else {
if (workInProgressHook.next === null) {
isReRender = false
// Append to the end of the list
workInProgressHook = workInProgressHook.next = createHook()
return (workInProgressHook = workInProgressHook.next = createHook())
} else {
// There's already a work-in-progress. Reuse it.
isReRender = true
workInProgressHook = workInProgressHook.next
return (workInProgressHook = workInProgressHook.next)
}
}
return workInProgressHook
}

export function renderWithHooks(
Expand Down Expand Up @@ -200,7 +189,7 @@ function useReducer<S, I, A>(
const dispatch: Dispatch<A> =
queue.dispatch || (queue.dispatch = dispatchAction.bind(null, id, queue))

if (isReRender && renderPhaseUpdates !== null) {
if (renderPhaseUpdates !== null) {
// This is a re-render. Apply the new render phase updates to the previous
// current hook.
// Render phase updates are stored in a map of queue -> linked list
Expand Down Expand Up @@ -230,16 +219,11 @@ function useMemo<T>(nextCreate: () => T, deps: Array<mixed> | void | null): T {
workInProgressHook = createWorkInProgressHook()

const nextDeps = deps === undefined ? null : deps

if (workInProgressHook !== null) {
const prevState = workInProgressHook.memoizedState
if (prevState !== null) {
if (nextDeps !== null) {
const prevDeps = prevState[1]
if (areHookInputsEqual(nextDeps, prevDeps)) {
return prevState[0]
}
}
const prevState = workInProgressHook.memoizedState
if (prevState !== null && nextDeps !== null) {
const prevDeps = prevState[1]
if (areHookInputsEqual(nextDeps, prevDeps)) {
return prevState[0]
}
}

Expand Down Expand Up @@ -297,8 +281,7 @@ function dispatchAction<A>(
}

function useCallback<T>(callback: T, deps: Array<mixed> | void | null): T {
// Callbacks are passed as they are in the server environment.
return callback
return useMemo(() => callback, deps)
}

function noop(): void {}
Expand Down

0 comments on commit 4e557aa

Please sign in to comment.