-
Notifications
You must be signed in to change notification settings - Fork 368
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
PathFunction
should require parameter
#255
Comments
Proposal: export declare function compile<P>(str: string, options?: ParseOptions & TokensToFunctionOptions): P extends object ? PathFunction<P> : PathFunctionWithoutParams;
export declare type PathFunctionWithoutParams = () => string;
export declare type PathFunction<P extends object> = (data: P) => string; |
Sounds good to me, I’ll consider this in the next release. No timeline for that release right now though. |
We may also utilize template literal types to infer path parameters from the path string. See the example. With the parameter inference, the developers would no longer have to specify the parameter types explicitly (which still opens room for error, regardless of the type being required or not). With explicit parameters type// Impossible state is a matter of human error.
const fn = p.compile<{ id: string }>('/user/:messageId')
fn({ id: 'foo' }) // OK while being an error state With inferred parameters typeconst fn = p.compile('/user/:messageId')
fn({ id: 'foo' }) // ERROR, unknown key "id"
fn({ messageId: 'foo' }) // OK The downside of such string inference is that it's available only since TypeScript 4.1. |
@blakeembrey, I'd like to take initiative in implementing this support. Please let me know which direction would you like to take: explicit required path parameters type or implicit inferred (see the justification above). |
I love the idea of being able to infer in typescript, I assumed it might be too difficult though. Let’s make it required and I can create a “next” branch so it can be in the next release and we can still fix bugs in the current release. |
At the very least I think we could do both, potentially use “unknown” for inferred keys and have it there as type safe validation that the interface type is valid? |
From my initial assessment, it doesn't require much to have parameters inferrence. If you don't mind, I'll open a pull request where I do both and we can discuss it there. Sounds good? |
Sounds great to me, thanks! Sorry for the delay, getting back through my backlog of open source issues now and promise I'll review your PR by the end of next week. |
Currently
PathFunction
is typed such that its parameter is optional, however this allows mistakes such as the following:The text was updated successfully, but these errors were encountered: