update ConstraintStrategy typing #180
Conversation
|
Which tests exactly need to be updated? For me it looks like it would make most sense to add a new tests for custom constraints with objects. ( like |
|
Adding new tests is totally fine |
|
We just need to test the types: https://github.com/delvedor/find-my-way/blob/master/test/types/router.test-d.ts |
index.d.ts
Outdated
| get(version: String) : Handler<V> | null, | ||
| set(version: String, store: Handler<V>) : void, | ||
| del(version: String) : void, | ||
| get(version: T) : Handler<V> | null, |
There was a problem hiding this comment.
Would you mind updating this variable to be named value or something less version specific? Not strictly a result of your change but it is a bit more confusing now that the type of this piece is generic
There was a problem hiding this comment.
Perhaps the name of the store variable should also be changed, as this has simply been taken over from the semver-store?
There was a problem hiding this comment.
Oh -- yep definitely, that's a handler not a store. Thanks for leaving it better than you found it!!
In #179 it was asked whether it is possible to use objects as values in custom constraints. After looking through the code, this should not be a problem. Only the typings have to be updated.
The default value of the generic could be
stringorany.anywould match the typing of the RouteOption.But
stringwould provide a backwards compatibility.