Skip to content

Commit

Permalink
fix thenable types & update another incorrect cache record (#55758)
Browse files Browse the repository at this point in the history
This is a continuation from #55690 but also properly types these functions so we can catch these easier
  • Loading branch information
ztanner committed Sep 21, 2023
1 parent ff7e4f4 commit e0b8d32
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 46 deletions.
13 changes: 8 additions & 5 deletions packages/next/src/client/components/layout-router.tsx
Expand Up @@ -27,6 +27,7 @@ import { RedirectBoundary } from './redirect-boundary'
import { NotFoundBoundary } from './not-found-boundary'
import { getSegmentValue } from './router-reducer/reducers/get-segment-value'
import { createRouterCacheKey } from './router-reducer/create-router-cache-key'
import { createRecordFromThenable } from './router-reducer/create-record-from-thenable'

/**
* Add refetch marker to router state at the point of the current layout segment.
Expand Down Expand Up @@ -378,11 +379,13 @@ function InnerLayoutRouter({

childNode = {
status: CacheStates.DATA_FETCH,
data: fetchServerResponse(
new URL(url, location.origin),
refetchTree,
context.nextUrl,
buildId
data: createRecordFromThenable(
fetchServerResponse(
new URL(url, location.origin),
refetchTree,
context.nextUrl,
buildId
)
),
subTreeData: null,
head:
Expand Down
9 changes: 7 additions & 2 deletions packages/next/src/client/components/promise-queue.ts
@@ -1,3 +1,5 @@
import type { ThenableRecord } from './router-reducer/router-reducer-types'

/*
This is a simple promise queue that allows you to limit the number of concurrent promises
that are running at any given time. It's used to limit the number of concurrent
Expand All @@ -7,7 +9,10 @@
export class PromiseQueue {
#maxConcurrency: number
#runningCount: number
#queue: Array<{ promiseFn: Promise<any>; task: () => void }>
#queue: Array<{
promiseFn: ThenableRecord<any> | Promise<any>
task: () => void
}>

constructor(maxConcurrency = 5) {
this.#maxConcurrency = maxConcurrency
Expand Down Expand Up @@ -45,7 +50,7 @@ export class PromiseQueue {
return taskPromise
}

bump(promiseFn: Promise<any>) {
bump(promiseFn: ThenableRecord<any> | Promise<any>) {
const index = this.#queue.findIndex((item) => item.promiseFn === promiseFn)

if (index > -1) {
Expand Down
Expand Up @@ -7,6 +7,10 @@ describe('createRecordFromThenable', () => {
expect(record.status).toBe('pending')
await thenable
expect(record.status).toBe('fulfilled')
// typeguard
if (record.status !== 'fulfilled') {
throw new Error('Invalid Type')
}
expect(record.value).toBe('success')
})

Expand All @@ -16,6 +20,10 @@ describe('createRecordFromThenable', () => {
expect(record.status).toBe('pending')
await thenable.catch(() => {})
expect(record.status).toBe('rejected')
expect(record.value).toBe('error')
// typeguard
if (record.status !== 'rejected') {
throw new Error('Invalid Type')
}
expect(record.reason).toBe('error')
})
})
@@ -1,8 +1,13 @@
import { ThenableRecord } from './router-reducer-types'

/**
* Create data fetching record for Promise.
*/
// TODO-APP: change `any` to type inference.
export function createRecordFromThenable(thenable: any) {

export function createRecordFromThenable<T>(
promise: PromiseLike<T>
): ThenableRecord<T> {
const thenable = promise as any
thenable.status = 'pending'
thenable.then(
(value: any) => {
Expand All @@ -14,7 +19,7 @@ export function createRecordFromThenable(thenable: any) {
(err: any) => {
if (thenable.status === 'pending') {
thenable.status = 'rejected'
thenable.value = err
thenable.reason = err
}
}
)
Expand Down
Expand Up @@ -29,7 +29,7 @@ import { callServer } from '../../app-call-server'
import { PrefetchKind } from './router-reducer-types'
import { hexHash } from '../../../shared/lib/hash'

type FetchServerResponseResult = [
export type FetchServerResponseResult = [
flightData: FlightData,
canonicalUrlOverride: URL | undefined
]
Expand Down
@@ -1,19 +1,23 @@
import React from 'react'
import { fetchServerResponse } from './fetch-server-response'
import type { FetchServerResponseResult } from './fetch-server-response'
import { fillCacheWithDataProperty } from './fill-cache-with-data-property'
import {
CacheStates,
CacheNode,
} from '../../../shared/lib/app-router-context.shared-runtime'
import { createRecordFromThenable } from './create-record-from-thenable'
import { ThenableRecord } from './router-reducer-types'
describe('fillCacheWithDataProperty', () => {
it('should add data property', () => {
const fetchServerResponseMock: jest.Mock<
ReturnType<typeof fetchServerResponse>
ThenableRecord<FetchServerResponseResult>
> = jest.fn(() =>
Promise.resolve([
/* TODO-APP: replace with actual FlightData */ '',
undefined,
])
createRecordFromThenable(
Promise.resolve([
/* TODO-APP: replace with actual FlightData */ '',
undefined,
])
)
)
const pathname = '/dashboard/settings'
const segments = pathname.split('/')
Expand Down Expand Up @@ -95,7 +99,9 @@ describe('fillCacheWithDataProperty', () => {
</React.Fragment>,
},
"dashboard" => Object {
"data": Promise {},
"data": Promise {
"status": "pending",
},
"parallelRoutes": Map {},
"status": "DATAFETCH",
"subTreeData": null,
Expand Down
@@ -1,10 +1,11 @@
import type { FetchServerResponseResult } from './fetch-server-response'
import type { ThenableRecord } from './router-reducer-types'
import { FlightSegmentPath } from '../../../server/app-render/types'
import {
CacheNode,
CacheStates,
} from '../../../shared/lib/app-router-context.shared-runtime'
import { createRouterCacheKey } from './create-router-cache-key'
import { fetchServerResponse } from './fetch-server-response'

/**
* Kick off fetch based on the common layout between two routes. Fill cache with data property holding the in-progress fetch.
Expand All @@ -13,7 +14,7 @@ export function fillCacheWithDataProperty(
newCache: CacheNode,
existingCache: CacheNode,
flightSegmentPath: FlightSegmentPath,
fetchResponse: () => ReturnType<typeof fetchServerResponse>,
fetchResponse: () => ThenableRecord<FetchServerResponseResult>,
bailOnParallelRoutes: boolean = false
): { bailOptimistic: boolean } | undefined {
const isLastEntry = flightSegmentPath.length <= 2
Expand Down
Expand Up @@ -23,6 +23,6 @@ describe('readRecordValue', () => {
}
})()
expect(result.status).toBe('rejected')
expect(result.value).toBe('failed')
expect(result.reason).toBe('failed')
})
})
@@ -1,10 +1,10 @@
import type { ThenableRecord } from './router-reducer-types'

/**
* Read record value or throw Promise if it's not resolved yet.
*/
export function readRecordValue<T>(thenable: Promise<T>): T {
// @ts-expect-error TODO: fix type
export function readRecordValue<T>(thenable: ThenableRecord<T>): T {
if (thenable.status === 'fulfilled') {
// @ts-expect-error TODO: fix type
return thenable.value
} else {
throw thenable
Expand Down
Expand Up @@ -6,7 +6,10 @@ import type {
FlightRouterState,
FlightSegmentPath,
} from '../../../../server/app-render/types'
import { fetchServerResponse } from '../fetch-server-response'
import {
FetchServerResponseResult,
fetchServerResponse,
} from '../fetch-server-response'
import { createRecordFromThenable } from '../create-record-from-thenable'
import { readRecordValue } from '../read-record-value'
import { createHrefFromUrl } from '../create-href-from-url'
Expand All @@ -22,6 +25,7 @@ import {
PrefetchKind,
ReadonlyReducerState,
ReducerState,
ThenableRecord,
} from '../router-reducer-types'
import { handleMutable } from '../handle-mutable'
import { applyFlightData } from '../apply-flight-data'
Expand Down Expand Up @@ -78,7 +82,7 @@ function addRefetchToLeafSegments(
currentCache: CacheNode,
flightSegmentPath: FlightSegmentPath,
treePatch: FlightRouterState,
data: () => ReturnType<typeof fetchServerResponse>
data: () => ThenableRecord<FetchServerResponseResult>
) {
let appliedPatch = false

Expand Down Expand Up @@ -159,7 +163,7 @@ export function navigateReducer(
temporaryCacheNode.subTreeData = state.cache.subTreeData
temporaryCacheNode.parallelRoutes = new Map(state.cache.parallelRoutes)

let data: ReturnType<typeof createRecordFromThenable> | undefined
let data: ThenableRecord<FetchServerResponseResult> | null = null

const fetchResponse = () => {
if (!data) {
Expand Down Expand Up @@ -199,7 +203,7 @@ export function navigateReducer(
mutable.canonicalUrl = href

state.prefetchCache.set(createHrefFromUrl(url, false), {
data: createRecordFromThenable(Promise.resolve(data)),
data: data ? createRecordFromThenable(Promise.resolve(data)) : null,
// this will make sure that the entry will be discarded after 30s
kind: PrefetchKind.TEMPORARY,
prefetchTime: Date.now(),
Expand Down
Expand Up @@ -166,16 +166,19 @@ export function serverActionReducer(
mutable.globalMutable.pendingNavigatePath &&
mutable.globalMutable.pendingNavigatePath !== href
) {
mutable.inFlightServerAction.then(() => {
if (mutable.actionResultResolved) return

// if the server action resolves after a navigation took place,
// reset ServerActionMutable values & trigger a refresh so that any stale data gets updated
mutable.inFlightServerAction = null
mutable.globalMutable.pendingNavigatePath = undefined
mutable.globalMutable.refresh()
mutable.actionResultResolved = true
})
mutable.inFlightServerAction.then(
() => {
if (mutable.actionResultResolved) return

// if the server action resolves after a navigation took place,
// reset ServerActionMutable values & trigger a refresh so that any stale data gets updated
mutable.inFlightServerAction = null
mutable.globalMutable.pendingNavigatePath = undefined
mutable.globalMutable.refresh()
mutable.actionResultResolved = true
},
() => {}
)

return state
}
Expand Down Expand Up @@ -305,7 +308,7 @@ export function serverActionReducer(
} catch (e: any) {
if (e.status === 'rejected') {
if (!mutable.actionResultResolved) {
reject(e.value)
reject(e.reason)
mutable.actionResultResolved = true
}

Expand Down
Expand Up @@ -4,7 +4,12 @@ import type {
FlightData,
FlightSegmentPath,
} from '../../../server/app-render/types'
import { fetchServerResponse } from './fetch-server-response'
import type {
FulfilledThenable,
PendingThenable,
RejectedThenable,
} from 'react'
import type { FetchServerResponseResult } from './fetch-server-response'

export const ACTION_REFRESH = 'refresh'
export const ACTION_NAVIGATE = 'navigate'
Expand Down Expand Up @@ -46,7 +51,7 @@ export interface Mutable {
}

export interface ServerActionMutable extends Mutable {
inFlightServerAction?: Promise<any> | null
inFlightServerAction?: ThenableRecord<any> | null
actionResultResolved?: boolean
}

Expand Down Expand Up @@ -217,7 +222,7 @@ export type FocusAndScrollRef = {

export type PrefetchCacheEntry = {
treeAtTimeOfPrefetch: FlightRouterState
data: ReturnType<typeof fetchServerResponse> | null
data: ThenableRecord<FetchServerResponseResult> | null
kind: PrefetchKind
prefetchTime: number
lastUsedTime: number | null
Expand Down Expand Up @@ -278,3 +283,8 @@ export type ReducerActions = Readonly<
| FastRefreshAction
| ServerActionAction
>

export type ThenableRecord<T> =
| PendingThenable<T>
| RejectedThenable<T>
| FulfilledThenable<T>
@@ -1,10 +1,11 @@
'use client'

import {
import type {
FocusAndScrollRef,
PrefetchKind,
ThenableRecord,
} from '../../client/components/router-reducer/router-reducer-types'
import type { fetchServerResponse } from '../../client/components/router-reducer/fetch-server-response'
import type { FetchServerResponseResult } from '../../client/components/router-reducer/fetch-server-response'
import type {
FlightRouterState,
FlightData,
Expand All @@ -29,7 +30,7 @@ export type CacheNode =
/**
* In-flight request for this node.
*/
data: ReturnType<typeof fetchServerResponse> | null
data: ThenableRecord<FetchServerResponseResult> | null
head?: React.ReactNode
/**
* React Component for this node.
Expand Down

0 comments on commit e0b8d32

Please sign in to comment.