-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(urlMatcherFactory): fix configuring a parameter type by name in a… #2296
Conversation
@@ -917,10 +921,15 @@ function $UrlMatcherFactory() { | |||
} | |||
|
|||
function getType(config, urlType, location) { | |||
if (config.type && urlType) throw new Error("Param '"+id+"' has two type configurations."); | |||
if (config.type && urlType && config.type !== urlType) throw new Error("Param '"+id+"' has two type configurations."); |
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.
whops, missed that. I will fix and force push
9803883
to
6bfd4d6
Compare
@@ -122,7 +122,11 @@ function UrlMatcher(pattern, config, parentMatcher) { | |||
cfg = config.params[id]; | |||
segment = pattern.substring(last, m.index); | |||
regexp = isSearch ? m[4] : m[4] || (m[1] == '*' ? '.*' : null); | |||
type = $$UMFP.type(regexp || "string") || inherit($$UMFP.type("string"), { pattern: new RegExp(regexp, config.caseInsensitive ? 'i' : undefined) }); |
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.
defer applying the default path/query param type of "string" until the getType
function.
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.
Yes this was the bit that was setting things as type string for me. I didn't fully understand all the code so I tried to confine my changes to getType
as to limit impact.
}); | ||
|
||
it("should throw an error if a param type is declared twice", function () { | ||
try { |
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.
Don't you want expect(...).toThrow(...)
?
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.
updated
… params block closes #2294
6bfd4d6
to
e401024
Compare
This looks 👍 to me, and should translate 1:1 onto the params branch. |
fix(urlMatcherFactory): fix configuring a parameter type by name in a…
… params block
closes #2294