Accept undefined on optional RouteDefinition properties - #599
Merged
Conversation
Under exactOptionalPropertyTypes, an optional property only admits an
explicit undefined when its type says so. fileRoutes() maps manifest
leaves to { component, children: undefined } (a required key, mirroring
the runtime), which RouteDefinition's plain optional props rejected. Add
| undefined to every optional property of RouteDefinition, DefinedRoute,
FileRouteConfig and the defineRoute/defineFileRoute inputs, and make the
one presence check in createBranches (hasOwnProperty('path')) treat an
explicit path: undefined as pathless like an absent key.
Adds a compile-time regression test that type-checks a fixture shaped
like filesystem-routing's generated pageRoutes declaration with the flag
on, reporting only the fixture's diagnostics.
🦋 Changeset detectedLatest commit: 12a2500 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #598
With
exactOptionalPropertyTypes: true,createRouter({ routes: fileRoutes(pageRoutes) })fails againstfilesystem-routing's generatedvirtual:file-routestypes.Root cause is in the router, not the generator:
FileRouteFrom<E>(in@solidjs/router/fs) deliberately maps a leaf entry to a requiredchildren: undefined/component: undefinedkey — mirroring whatfileRoutes()emits at runtime — andRouteDefinition's optional properties wereprop?: Twithout| undefined, which the flag rejects. The generator'schildren?: undefinedfor absent children is fine on its own and already matchedFileRouteEntry's input types.Changes
RouteDefinition: every optional property (path,matchFilters,preload,children,component,search,info) now admits| undefined.DefinedRouteand thedefineRouteoverload inputs,FileRouteConfigand thedefineFileRouteconfig,FileRouteEagerRef.src.createBranchesusedhasOwnProperty("path")to default a pathless route to""; an explicitpath: undefinedwould have slipped through. It now checksdef.path === undefined. Everything else already treated absent andundefinedalike.test/exact-optional-types.spec.tscompilestest/fixtures/exact-optional-routes.tsthrough the compiler API withexactOptionalPropertyTypes: true(reporting only the fixture's diagnostics — the source itself does not opt into the flag, 19 unrelated errors). The fixture mirrors the plugin's generatedpageRoutesdeclaration and pushes it throughfileRoutes(),createRouter()(typedpathsintact),defineRoute, anddefineFileRoute. 7 errors before, 0 after.Verified against a fresh
/tmpproject (solid-js@2.0.0-rc.7,@solidjs/router@2.0.0-next.22,filesystem-routing@0.3.0,@solidjs/vite-plugin@3.0.0-next.38) with the flag on:tscreproduces the issue's error against the published package and passes against thepnpm packtarball of this branch.pnpm build,test:types, jsdom + server vitest suites, andattw --pack . --profile esm-onlyare all green.Made with Cursor