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.

Sign up for GitHub

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

Jump to bottom

Defer again after initial defer has been completed #110

Closed
wants to merge 4 commits into from
Closed

Defer again after initial defer has been completed #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2123c2
Always set deferring to true when pushing new defer update function
AdamRamberg Jan 17, 2023
ee58b05
Added unit test for when a child defers again after initial defer has…
AdamRamberg Jan 17, 2023
7065c5b
* Fix test according to feedback
AdamRamberg Jan 17, 2023
6d48a13
Clean up unit tests for deferred mount
AdamRamberg Jan 19, 2023
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
155 changes: 99 additions & 56 deletions packages/@react-facet/deferred-mount/src/index.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,27 @@ interface Cb {
frameId: number
}

jest.useFakeTimers()

const frames: (() => void)[] = []
const requestSpy = jest.spyOn(window, 'requestAnimationFrame').mockImplementation((frameRequest) => {
const id = idSeed++
const cb = () => {
frameRequest(id)
}
frames.push(cb)
return id
})

const runRaf = () => {
const cb = frames.pop()
if (cb != null) act(() => cb())
}

afterEach(() => {
requestSpy.mockClear()
})

describe('DeferredMount', () => {
it('renders immediately if we dont have a provider', () => {
const { container } = render(
Expand All @@ -25,9 +46,69 @@ describe('DeferredMount', () => {
)
expect(container.firstChild).toContainHTML('<div>Should be rendered</div>')
})

it('defers again after initial defer has completed', () => {
const DeferConditionally: React.FC<{ mountDeferred: boolean }> = ({ mountDeferred }) => {
const isDeferringFacet = useIsDeferring()
return (
<>
Copy link
Contributor

@marlonicus marlonicus Jan 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe add a print out of the useIsDeffering() hook value so you can verify that it's being set correctly, and then later run some expects on it. wdyt?

Suggested change
<>
<>
<p>isDeferring: ${ useIsDeferring() }</p>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! I fixed it in my latest commit. Tried to follow the convention already in place in other tests for checking this.

<fast-text
text={useFacetMap((isDeferring) => (isDeferring ? 'deferring' : 'done'), [], [isDeferringFacet])}
/>
{mountDeferred && (
<DeferredMount>
<p>Conditionally rendered</p>
</DeferredMount>
)}
</>
)
}

const { container, rerender } = render(
<DeferredMountProvider>
<DeferConditionally mountDeferred={false} />
</DeferredMountProvider>,
)

expect(container).toContainHTML('deferring')
expect(container).not.toContainHTML('<p>Conditionally rendered</p>')

runRaf()
expect(container).toContainHTML('done')
expect(container).not.toContainHTML('<p>Conditionally rendered</p>')

rerender(
<DeferredMountProvider>
<DeferConditionally mountDeferred />
</DeferredMountProvider>,
)

expect(container).toContainHTML('deferring')
expect(container).not.toContainHTML('<p>Conditionally rendered</p>')

runRaf()
expect(container).toContainHTML('done')
expect(container).toContainHTML('<p>Conditionally rendered</p>')
})
})

describe('DeferredMountWithCallback', () => {
const MOUNT_COMPLETION_DELAY = 1000

const MockDeferredComponent = ({ index }: { index: number }) => {
const triggerMountComplete = useNotifyMountComplete()

useEffect(() => {
const id = setTimeout(triggerMountComplete, MOUNT_COMPLETION_DELAY)

return () => {
clearTimeout(id)
}
}, [triggerMountComplete, index])

return <div>Callback{index}</div>
}

it('renders immediately if we dont have a provider', () => {
const { container } = render(
<DeferredMountWithCallback>
Expand All @@ -37,61 +118,26 @@ describe('DeferredMountWithCallback', () => {
expect(container.firstChild).toContainHTML('<div>Should be rendered</div>')
})

it('waits until previous deferred callback finishes', async () => {
jest.useFakeTimers()

const frames: (() => void)[] = []
const requestSpy = jest.spyOn(window, 'requestAnimationFrame').mockImplementation((frameRequest) => {
const id = idSeed++
const cb = () => {
frameRequest(id)
}
frames.push(cb)
return id
})

const runRaf = () => {
const cb = frames.pop()
if (cb != null) act(() => cb())
}

const MOUNT_COMPLETION_DELAY = 1000

const MockDeferredComponent = ({ index }: { index: number }) => {
const triggerMountComplete = useNotifyMountComplete()

useEffect(() => {
const id = setTimeout(triggerMountComplete, MOUNT_COMPLETION_DELAY)

return () => {
clearTimeout(id)
}
}, [triggerMountComplete, index])

return <div>Callback{index}</div>
}

const SampleComponent = () => {
const isDeferringFacet = useIsDeferring()
const SampleComponent = () => {
const isDeferringFacet = useIsDeferring()

return (
<>
<fast-text
text={useFacetMap((isDeferring) => (isDeferring ? 'deferring' : 'done'), [], [isDeferringFacet])}
/>
<DeferredMountWithCallback>
<MockDeferredComponent index={0} />
</DeferredMountWithCallback>
<DeferredMountWithCallback>
<MockDeferredComponent index={1} />
</DeferredMountWithCallback>
<DeferredMountWithCallback>
<MockDeferredComponent index={2} />
</DeferredMountWithCallback>
</>
)
}
return (
<>
<fast-text text={useFacetMap((isDeferring) => (isDeferring ? 'deferring' : 'done'), [], [isDeferringFacet])} />
<DeferredMountWithCallback>
<MockDeferredComponent index={0} />
</DeferredMountWithCallback>
<DeferredMountWithCallback>
<MockDeferredComponent index={1} />
</DeferredMountWithCallback>
<DeferredMountWithCallback>
<MockDeferredComponent index={2} />
</DeferredMountWithCallback>
</>
)
}

it('waits until previous deferred callback finishes', async () => {
const { container } = render(
<DeferredMountProvider>
<SampleComponent />
Expand Down Expand Up @@ -128,9 +174,6 @@ describe('DeferredMountWithCallback', () => {
jest.advanceTimersByTime(MOUNT_COMPLETION_DELAY)
runRaf()
expect(container).toContainHTML('done')

jest.useRealTimers()
requestSpy.mockRestore()
})
})

Expand Down
14 changes: 6 additions & 8 deletions packages/@react-facet/deferred-mount/src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,14 @@ export function InnerDeferredMountProvider({
frameTimeBudget = DEFAULT_FRAME_TIME_BUDGET,
}: DeferredMountProviderProps) {
const [isDeferring, setIsDeferring] = useFacetState(true)
const [requestingToRun, setRequestingToRun] = useFacetState(false)
const waitingForMountCallback = useRef(false)

const deferredMountsRef = useRef<UpdateFn[]>([])

const pushDeferUpdateFunction = useCallback(
(updateFn: UpdateFn) => {
// Causes a re-render of this component that will kick-off the effect below
setRequestingToRun(true)
setIsDeferring(true)

deferredMountsRef.current.push(updateFn)

Expand All @@ -69,14 +68,14 @@ export function InnerDeferredMountProvider({
}
}
},
[setRequestingToRun],
[setIsDeferring],
)

useFacetEffect(
(requestingToRun) => {
(isDeferring) => {
// Even if we are not considered to be running, we need to check if there is still
// work pending to be done. If there is... we still need to run this effect.
if (!requestingToRun && deferredMountsRef.current.length === 0 && !waitingForMountCallback.current) return
if (!isDeferring && deferredMountsRef.current.length === 0 && !waitingForMountCallback.current) return

const work = (startTimestamp: number) => {
const deferredMounts = deferredMountsRef.current
Expand Down Expand Up @@ -131,7 +130,6 @@ export function InnerDeferredMountProvider({

if (deferredMounts.length === 0 && !waitingForMountCallback.current) {
setIsDeferring(false)
setRequestingToRun(false)
}
}

Expand All @@ -141,8 +139,8 @@ export function InnerDeferredMountProvider({
window.cancelAnimationFrame(frameId)
}
},
[frameTimeBudget, setIsDeferring, setRequestingToRun],
[requestingToRun],
[frameTimeBudget, setIsDeferring],
[isDeferring],
)

return (
Expand Down