-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat(route-pattern): add createRouter
& urlpat
#10756
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.
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
base: main
Are you sure you want to change the base?
Conversation
createRouter
& urlpat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a native URLPattern integration and a lightweight router helper to route matching.
- Introduces urlpat to convert pathname-only RoutePattern definitions into native URLPattern for fast prefiltering.
- Adds createRouter with get, all, and handle APIs that prefer URLPattern when available and fall back to RoutePattern.match.
- Exposes urlpat and createRouter from the package index.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
File | Description |
---|---|
packages/route-pattern/src/lib/router.ts | Adds createRouter with route registration and request handling, leveraging URLPattern prefiltering. |
packages/route-pattern/src/lib/route-pattern.ts | Adds urlpat and helper to translate tokenized patterns to native URLPattern; includes new doc comments. |
packages/route-pattern/src/index.ts | Exports urlpat and createRouter from the package. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
export function urlpat(input:string|RoutePattern):URLPattern|null{ | ||
let Ctor =(globalThis as any).URLPattern |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the URLPattern type in the signature introduces a compile-time dependency on lib.dom, which may break consumers compiling for non-DOM targets. Consider a conditional or local alias to avoid requiring DOM types, e.g.: type NativeURLPattern = typeof globalThis extends { URLPattern: infer U } ? U : never and export function urlpat(...): NativeURLPattern | null.
export function urlpat(input:string|RoutePattern):URLPattern|null{ | |
let Ctor =(globalThis as any).URLPattern | |
type NativeURLPattern = typeof globalThis extends { URLPattern: infer U } ? U : never; | |
export function urlpat(input: string | RoutePattern): NativeURLPattern | null { | |
let Ctor = (globalThis as any).URLPattern |
Copilot uses AI. Check for mistakes.
import type {Params} from './params.ts' | ||
export type Method='GET'|'POST'|'PUT'|'DELETE'|'PATCH'|'HEAD'|'OPTIONS'|'*' | ||
export type Handler<P extends string>=(req:Request,params:Params<P>)=>Response | Promise<Response> | ||
type Route ={method:Method; pattern:RoutePattern<any>; up?:URLPattern; handler:Handler<any>} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same DOM typing concern as urlpat: up?: URLPattern will require lib.dom. Prefer a local alias (e.g., type NativeURLPattern = ReturnType extends infer R ? Exclude<R, null> : never) or a minimal structural type (e.g., { test(input: string | URL, baseURL?: string): boolean }) to keep the package environment-agnostic.
type Route ={method:Method; pattern:RoutePattern<any>; up?:URLPattern; handler:Handler<any>} | |
type URLPatternLike = { test(input: string | URL, baseURL?: string): boolean } | |
type Route ={method:Method; pattern:RoutePattern<any>; up?:URLPatternLike; handler:Handler<any>} |
Copilot uses AI. Check for mistakes.
// Anative URLPattern for pathname-only patterns (variables, wildcards) | ||
//eturns null if not supported or pattern uses enums or optionals or origin/search.... | ||
**/ | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docblock uses line comments inside a block and contains typos, making it harder to read. Please convert to proper JSDoc and clarify constraints, e.g.: /** Converts pathname-only RoutePattern definitions to native URLPattern. Returns null if URLPattern is unavailable or if the pattern includes origin, search, enums, or optionals. */
// Anative URLPattern for pathname-only patterns (variables, wildcards) | |
//eturns null if not supported or pattern uses enums or optionals or origin/search.... | |
**/ | |
* Converts pathname-only RoutePattern definitions to native URLPattern. | |
* Returns null if URLPattern is unavailable or if the pattern includes origin, search, enums, or optionals. | |
* | |
* @param input - A string or RoutePattern to convert. | |
* @returns A URLPattern instance, or null if constraints are not met. | |
*/ |
Copilot uses AI. Check for mistakes.
return true | ||
} | ||
/** | ||
// Anative URLPattern for pathname-only patterns (variables, wildcards) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct 'Anative' to 'A native'.
// Anative URLPattern for pathname-only patterns (variables, wildcards) | |
// A native URLPattern for pathname-only patterns (variables, wildcards) |
Copilot uses AI. Check for mistakes.
// Anative URLPattern for pathname-only patterns (variables, wildcards) | ||
//eturns null if not supported or pattern uses enums or optionals or origin/search.... | ||
**/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct 'eturns' to 'Returns' and remove the stray slashes for consistency with the doc style.
// Anative URLPattern for pathname-only patterns (variables, wildcards) | |
//eturns null if not supported or pattern uses enums or optionals or origin/search.... | |
**/ | |
* A native URLPattern for pathname-only patterns (variables, wildcards). | |
* Returns null if not supported or if the pattern uses enums, optionals, origin, or search. | |
*/ |
Copilot uses AI. Check for mistakes.
export function createRouter(){ | ||
const routes:Route[]=[] | ||
const api={ | ||
get<P extends string>(pattern:P,handler:Handler<P>){add('GET',pattern,handler);return api}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method union includes POST/PUT/DELETE/PATCH/etc., but the public API only exposes get and all. Consider adding per-verb helpers (post, put, delete, patch, head, options) or a generic on(method: Method, ...) to avoid surprising consumers.
get<P extends string>(pattern:P,handler:Handler<P>){add('GET',pattern,handler);return api}, | |
get<P extends string>(pattern:P,handler:Handler<P>){add('GET',pattern,handler);return api}, | |
post<P extends string>(pattern:P,handler:Handler<P>){add('POST',pattern,handler);return api}, | |
put<P extends string>(pattern:P,handler:Handler<P>){add('PUT',pattern,handler);return api}, | |
delete<P extends string>(pattern:P,handler:Handler<P>){add('DELETE',pattern,handler);return api}, | |
patch<P extends string>(pattern:P,handler:Handler<P>){add('PATCH',pattern,handler);return api}, | |
head<P extends string>(pattern:P,handler:Handler<P>){add('HEAD',pattern,handler);return api}, | |
options<P extends string>(pattern:P,handler:Handler<P>){add('OPTIONS',pattern,handler);return api}, |
Copilot uses AI. Check for mistakes.
First I would like to ask if exporting allowed? ...or if it need more expansions?
added urlpat to @remix-run/route-pattern. This return a native URLPattern for pathname only patterns (vars,etc). Added a createRouter helper inside route-pattern getter(pat,handler), all(pattern, handler), handle(request) will uses the concerned pattern when available, Fallback to RoutePattern.match otherwise
Exposes both urlpat and createRouter from the route-pattern package index.