Support for ? in paths #214
Conversation
| it('returns an object with the params', function () { | ||
| expect(Path.extractParams(pattern, '/archive')).toEqual({ name: undefined }); | ||
| expect(Path.extractParams(pattern, '/archive/')).toEqual({ name: undefined }); | ||
| expect(Path.extractParams(pattern, '/archive/foo')).toEqual({ name: 'foo' }); |
There was a problem hiding this comment.
what about /archivefoo?
mjackson
force-pushed
the
path-matching
branch
from
b3673c8
to
175b485
Compare
| case '(': | ||
| case ')': | ||
| case '^': | ||
| case '$': |
There was a problem hiding this comment.
are any of these chars even valid in a URL?
edit: well, I suppose . is…
There was a problem hiding this comment.
Yes, all except for ^ which should be URL-encoded. See RFC 3986. The thinking here was mainly to prevent them from appearing in the source of the RegExp.
There was a problem hiding this comment.
Right, I just meant that you wouldn't even write those as part of your URL patterns if URLs can't look like that. I could imagine it being valuable to support regex patterns in the future but this seems good for now. Do you also want to do []+|{}\ which are also regexp special chars?
There was a problem hiding this comment.
Yes, I should probably add those too. + will need some special handling because that's how we encode spaces in URLs.
mjackson
force-pushed
the
path-matching
branch
from
175b485
to
5004679
Compare
|
@spicyj All special RegExp chars are now escaped properly. |
|
|
||
| describe('when a pattern has multiple splats', function () { | ||
| it('returns the correct path', function () { | ||
| expect(Path.injectParams('/a/*/c/*', { splat: [ 'b', 'd' ] })).toEqual('/a/b/c/d'); |
There was a problem hiding this comment.
When would I use splats instead of params?
|
We have an app right now that uses the splat to match an arbitrary file path in a file browsing app.
This will match things like Issues are:
|
[changed] :param no longer matches . [added] Support for arrays in query strings Fixes #142
mjackson
force-pushed
the
path-matching
branch
from
5004679
to
6a552b9
Compare
|
ok, so now that I've gotten some sleep and am thinking about this clearly, here's what's going on:
So, all of the following paths work as you'd expect: Also, we should point out in the docs that |
|
closed by d5bd656 |
|
👍 I reworded the commit because the changelog only looks at subjects, not the commit message body. I wanted a message that says "hey ... lots changed" |
[changed] * in paths no longer matches .
[added] Support for arrays in query strings
Fixes #142