Inertia adapter(s) affected
JS package version
2.3.18
Backend stack (optional)
(Not applicable, the bug is in @inertiajs/core)
Describe the problem
In getPendingVisit
|
protected getPendingVisit(href: string | URL | UrlMethodPair, options: VisitOptions): PendingVisit { |
default values are correctly defined in the
mergedOptions object (typed as
Visit).
However, options parameters is typed as VisitOptions (which is Partial<Visit & VisitCallbacks>), which means TypeScript allows properties to be explicitly set to undefined. E.g. when an options object like { only: undefined } is spread into mergedOptions, the explicit undefined value overwrites the default [], producing an object whose runtime value contradicts its declared type (Visit), without exactOptionalPropertyTypes TypeScript does not flag this.
Later in the visit method, visit.only.length, visit.except.length, and visit.reset.length are accessed directly (introduced by PR #2843) without any null/undefined guard (which is fine because the type tell that these properties should be defined). This causes a runtime crash:
Uncaught TypeError: Cannot read properties of undefined (reading 'length')
It happens in practice when options are built dynamically (e.g. conditionally spreading partial option objects, or when a wrapper function passes through options that may be undefined). This bit me in production while migrating from 1.x to 2.x, which is how I ended up here 😅
Expected behavior: router.visit('/url', { only: undefined }) should behave the same as router.visit('/url', {}). The default value ([]) should not be overwritten by an explicit undefined. Otherwise, types should not allow this.
Possibles solutions:
- Strip
undefined values from options and configuredOptions before merging. This is the most pragmatic and simple fix.
- Enable
exactOptionalPropertyTypes in tsconfig.json to make TypeScript reject { only: undefined } at the type level. This is the "correct" TS solution but has a much larger blast radius (tried it, a lot of errors comes up, unrelated to this, although this does indeed involve other potential risks).
Steps to reproduce
import { router } from '@inertiajs/react' // or vue, or svelte
router.visit('/some-url', { only: undefined })
// => Uncaught TypeError: Cannot read properties of undefined (reading 'length')
The issue is reproducible on the latest main branch (v3).
I'm willing to open a PR if solution 1 or 2 is fine.
Inertia adapter(s) affected
JS package version
2.3.18
Backend stack (optional)
(Not applicable, the bug is in
@inertiajs/core)Describe the problem
In
getPendingVisitinertia/packages/core/src/router.ts
Line 621 in b58c5bf
default values are correctly defined in the
mergedOptionsobject (typed asVisit).However,
optionsparameters is typed asVisitOptions(which isPartial<Visit & VisitCallbacks>), which means TypeScript allows properties to be explicitly set toundefined. E.g. when anoptionsobject like{ only: undefined }is spread intomergedOptions, the explicitundefinedvalue overwrites the default[], producing an object whose runtime value contradicts its declared type (Visit), withoutexactOptionalPropertyTypesTypeScript does not flag this.Later in the
visitmethod,visit.only.length,visit.except.length, andvisit.reset.lengthare accessed directly (introduced by PR #2843) without any null/undefined guard (which is fine because the type tell that these properties should be defined). This causes a runtime crash:It happens in practice when options are built dynamically (e.g. conditionally spreading partial option objects, or when a wrapper function passes through options that may be
undefined). This bit me in production while migrating from 1.x to 2.x, which is how I ended up here 😅Expected behavior:
router.visit('/url', { only: undefined })should behave the same asrouter.visit('/url', {}). The default value ([]) should not be overwritten by an explicitundefined. Otherwise, types should not allow this.Possibles solutions:
undefinedvalues fromoptionsandconfiguredOptionsbefore merging. This is the most pragmatic and simple fix.exactOptionalPropertyTypesintsconfig.jsonto make TypeScript reject{ only: undefined }at the type level. This is the "correct" TS solution but has a much larger blast radius (tried it, a lot of errors comes up, unrelated to this, although this does indeed involve other potential risks).Steps to reproduce
The issue is reproducible on the latest
mainbranch (v3).I'm willing to open a PR if solution 1 or 2 is fine.