-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Upgrade to Babel 6 #3362
Upgrade to Babel 6 #3362
Conversation
@@ -94,37 +94,29 @@ export function matchPattern(pattern, pathname) { | |||
} | |||
|
|||
const match = pathname.match(new RegExp(`^${regexpSource}`, 'i')) | |||
if (match == null) { | |||
return null |
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.
Are we sure we want to change the possible output signature of an exported 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.
It's explicitly not part of the public 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.
I mean for internal usage. At least make this into:
return {
remainingPathname: null,
paramNames: null,
paramValues: null
}
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.
I updated all consumers of matchPattern
. It's a lot clearer to check for a null
response value than check for e.g. matched.paramNames == null
.
Maybe this should be against |
It lets us drop |
Eh, OK. It's your funeral if this doesn't work :P |
It turns out what I was doing with the output of
matchPattern
was an abuse and only worked in loose mode. Granted, we're still using loose mode, but we might as well not abuse ES6 semantics.