Skip to content

Commit

Permalink
[form] allow turning prefetching off (#68305)
Browse files Browse the repository at this point in the history
Adds a `prefetch` prop to Form. It mirrors how Link's `prefetch` works,
but it only allows `prefetch={false}` (no prefetching) and
`prefetch={null}` ("auto" prefetch, the default). `prefetch={true}` does
not make sense for Form.

I don't expect this to be used much, because prefetching is one of the
main parts of Form's functionality, but i think it's good to have the
option to disable prefetching (even if mostly as an escape hatch)
  • Loading branch information
lubieowoce authored Sep 27, 2024
1 parent 9377b15 commit 3a909e2
Show file tree
Hide file tree
Showing 3 changed files with 300 additions and 5 deletions.
37 changes: 32 additions & 5 deletions packages/next/src/client/form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ type InternalFormProps = {
* - If `action` is a function, it will be called when the form is submitted. See the [React docs](https://react.dev/reference/react-dom/components/form#props) for more.
*/
action: NonNullable<HTMLFormProps['action']>
/**
* Controls how the route specified by `action` is prefetched.
* Any `<Form />` that is in the viewport (initially or through scroll) will be prefetched.
* Prefetch can be disabled by passing `prefetch={false}`. Prefetching is only enabled in production.
*
* Options:
* - `null` (default): For statically generated pages, this will prefetch the full React Server Component data. For dynamic pages, this will prefetch up to the nearest route segment with a [`loading.js`](https://nextjs.org/docs/app/api-reference/file-conventions/loading) file. If there is no loading file, it will not fetch the full tree to avoid fetching too much data.
* - `false`: This will not prefetch any data.
*
* @defaultValue `null`
*/
prefetch?: false | null
/**
* Whether submitting the form should replace the current `history` state instead of adding a new url into the stack.
* Only valid if `action` is a string.
Expand All @@ -45,12 +57,21 @@ export type FormProps<RouteInferType = any> = InternalFormProps
export default function Form({
replace,
scroll,
prefetch: prefetchProp = null,
ref: externalRef,
...props
}: FormProps) {
const actionProp = props.action
const isNavigatingForm = typeof actionProp === 'string'

if (process.env.NODE_ENV === 'development') {
if (!(prefetchProp === false || prefetchProp === null)) {
console.error('The `prefetch` prop of <Form> must be `false` or `null`')
}
}
const prefetch =
prefetchProp === false || prefetchProp === null ? prefetchProp : null

for (const key of DISALLOWED_FORM_PROPS) {
if (key in props) {
if (process.env.NODE_ENV === 'development') {
Expand Down Expand Up @@ -83,18 +104,19 @@ export default function Form({
return
}

if (!isVisible) {
const isPrefetchEnabled = prefetch === null

if (!isVisible || !isPrefetchEnabled) {
return
}

try {
// TODO: do we need to take the current field values here?
// or are we assuming that queryparams can't affect this (but what about rewrites)?
router.prefetch(actionProp, { kind: PrefetchKind.AUTO })
const prefetchKind = PrefetchKind.AUTO
router.prefetch(actionProp, { kind: prefetchKind })
} catch (err) {
console.error(err)
}
}, [isNavigatingForm, isVisible, actionProp, router])
}, [isNavigatingForm, isVisible, actionProp, prefetch, router])

if (!isNavigatingForm) {
if (process.env.NODE_ENV === 'development') {
Expand All @@ -106,6 +128,11 @@ export default function Form({
' `router.replace()` - https://nextjs.org/docs/app/api-reference/functions/use-router#userouter\n'
)
}
if (prefetchProp !== undefined) {
console.error(
'Passing `prefetch` to a <Form> whose `action` is a function has no effect.'
)
}
}
return <form {...props} ref={ownRef} />
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as React from 'react'
import Form from 'next/form'

export default function Home() {
return (
<Form action="/search" prefetch={false} id="search-form">
<input name="query" />
<button type="submit">Submit</button>
</Form>
)
}
257 changes: 257 additions & 0 deletions test/e2e/app-dir/next-form/default/next-form-prefetch.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
import { nextTestSetup, isNextDev } from 'e2e-utils'
import {
NEXT_ROUTER_PREFETCH_HEADER,
NEXT_RSC_UNION_QUERY,
RSC_HEADER,
} from 'next/src/client/components/app-router-headers'
import type { Route, Page, Request as PlaywrightRequest } from 'playwright'
import { WebdriverOptions } from '../../../../lib/next-webdriver'
import { retry } from '../../../../lib/next-test-utils'

const _describe =
// prefetching is disabled in dev.
isNextDev ||
// FIXME(form): currently failing in PPR, unflag this when https://github.com/vercel/next.js/pull/70532 is ready
process.env.__NEXT_EXPERIMENTAL_PPR
? describe.skip
: describe

_describe('app dir - form prefetching', () => {
const { next } = nextTestSetup({
files: __dirname,
})

it("should prefetch a loading state for the form's target", async () => {
const interceptor = new RequestInterceptor({
pattern: '**/search*',
requestFilter: async (request: PlaywrightRequest) => {
// only capture RSC requests that *aren't* prefetches
const headers = await request.allHeaders()
const isRSC = headers[RSC_HEADER.toLowerCase()] === '1'
const isPrefetch = !!headers[NEXT_ROUTER_PREFETCH_HEADER.toLowerCase()]
return isRSC && !isPrefetch
},
log: true,
})

const session = await next.browser('/forms/basic', {
beforePageLoad: interceptor.beforePageLoad,
})

await session.waitForIdleNetwork()
interceptor.start()

const searchInput = await session.elementByCss('input[name="query"]')
const query = 'my search'
await searchInput.fill(query)

const submitButton = await session.elementByCss('[type="submit"]')
await submitButton.click()

const targetHref = '/search?' + new URLSearchParams({ query })

// we're blocking requests, so the navigation won't go through,
// but we should still see the prefetched loading state
expect(await session.waitForElementByCss('#loading').text()).toBe(
'Loading...'
)

// We only resolve the dynamic request after we've confirmed loading exists,
// to avoid a race where the dynamic request handles the loading state instead.
const navigationRequest = interceptor.pendingRequests.get(targetHref)
expect(navigationRequest).toBeDefined()
// allow the navigation to continue
await navigationRequest.resolve()

const result = await session.waitForElementByCss('#search-results').text()
expect(result).toInclude(`query: "${query}"`)
})

it('should not prefetch when prefetch is set to false`', async () => {
const interceptor = new RequestInterceptor({
pattern: '**/search*',
requestFilter: async (request: PlaywrightRequest) => {
// capture all RSC requests, prefetch or not
const headers = await request.allHeaders()
const isRSC = headers[RSC_HEADER.toLowerCase()] === '1'
return isRSC
},
log: true,
})

interceptor.start()

const session = await next.browser('/forms/prefetch-false', {
beforePageLoad: interceptor.beforePageLoad,
})

await session.waitForIdleNetwork()

// no prefetches should have been captured
expect(interceptor.pendingRequests).toBeEmpty()

const searchInput = await session.elementByCss('input[name="query"]')
const query = 'my search'
await searchInput.fill(query)

const submitButton = await session.elementByCss('[type="submit"]')
await submitButton.click()

const targetHref = '/search?' + new URLSearchParams({ query })

// wait for the pending request to appear
const navigationRequest = await retry(
async () => {
const request = interceptor.pendingRequests.get(targetHref)
expect(request).toBeDefined()
return request
},
undefined,
100
)

// the loading state should not be visible, because we didn't prefetch it
expect(await session.elementsByCss('#loading')).toEqual([])

// allow the navigation to continue
navigationRequest.resolve()

// now, the loading state should stream in dynamically
expect(await session.waitForElementByCss('#loading').text()).toBe(
'Loading...'
)

const result = await session.waitForElementByCss('#search-results').text()
expect(result).toInclude(`query: "${query}"`)
})
})

// =====================================================================

type PlaywrightRoutePattern = Parameters<Page['route']>[0]
type PlaywrightRouteOptions = Parameters<Page['route']>[2]
type BeforePageLoadFn = NonNullable<WebdriverOptions['beforePageLoad']>

type RequestInterceptorOptions = {
pattern: PlaywrightRoutePattern
requestFilter?: (request: PlaywrightRequest) => boolean | Promise<boolean>
timeout?: number
log?: boolean
routeOptions?: PlaywrightRouteOptions
}

type InterceptedRequest = {
request: PlaywrightRequest
resolve(): Promise<void>
}

class RequestInterceptor {
pendingRequests: Map<string, InterceptedRequest> = new Map()
isEnabled = false
constructor(private opts: RequestInterceptorOptions) {}

private getRequestKey(request: PlaywrightRequest) {
// could cause issues for a generic util,
// but it's fine for the purposes of intercepting navigations.
// if we wanted this to be more generic, we could expose a `getRequest(...)` method
// to allow partial matching (e.g. by pathname + search only)
const url = new URL(request.url())
url.searchParams.delete(NEXT_RSC_UNION_QUERY)
return url.pathname + url.search
}

start() {
this.isEnabled = true
}

stop() {
this.isEnabled = false
}

beforePageLoad: BeforePageLoadFn = (page: Page) => {
const { opts } = this
page.route(
opts.pattern,
async (route: Route, request) => {
if (!this.isEnabled) {
return route.continue()
}

const shouldIntercept = opts.requestFilter
? opts.requestFilter(request)
: true

const url = request.url()

if (!shouldIntercept) {
if (opts.log) {
console.log('[interceptor] not intercepting:', url)
}
return route.continue()
}

const requestKey = this.getRequestKey(request)

if (opts.log) {
console.log(
'[interceptor] intercepting request:',
url,
`(key: ${requestKey})`
)
}

if (this.pendingRequests.has(requestKey)) {
throw new Error(
`Interceptor received duplicate request (key: ${JSON.stringify(requestKey)}, url: ${JSON.stringify(url)})`
)
}

// Create a promise that will be resolved by the later test code
const blocked = promiseWithResolvers<void>()

this.pendingRequests.set(requestKey, {
request,
resolve: async () => {
if (opts.log) {
console.log('[interceptor] resolving intercepted request:', url)
}
this.pendingRequests.delete(requestKey)
await route.continue()
// wait a moment to ensure the response is received
await new Promise((res) => setTimeout(res, 500))
blocked.resolve()
},
})

// Await the promise to effectively stall the request
const timeout = opts.timeout ?? 5000
await Promise.race([
blocked.promise,
timeoutPromise(
opts.timeout ?? 5000,
`Intercepted request for '${url}' was not resolved within ${timeout}ms`
),
])
},
opts.routeOptions
)
}
}

function promiseWithResolvers<T>() {
let resolve: (value: T) => void = undefined!
let reject: (error: unknown) => void = undefined!
const promise = new Promise((_resolve, _reject) => {
resolve = _resolve
reject = _reject
})
return { promise, resolve, reject }
}

function timeoutPromise(duration: number, message = 'Timeout') {
return new Promise<never>((_, reject) =>
AbortSignal.timeout(duration).addEventListener('abort', () =>
reject(new Error(message))
)
)
}

0 comments on commit 3a909e2

Please sign in to comment.