path-to-regexp icon indicating copy to clipboard operation
path-to-regexp copied to clipboard

Optional group with param compile

Open LKay opened this issue 7 years ago • 14 comments

There are some issues: #95, #106 but I'm not sure if that is the same case.

I want to achieve something like optional group matching based on presence of a parameter and being able to compile path back.

I'd like to define a path like:

(/first/:one)?/second

And the first segment will be returned only if :one parameter is present:

const toPath = pathToRegexp.compile("(/first/:one)?/second");

toPath(); // returns `/second`
toPath({ one : 1 }); // returns `/first/1/second`

Ideally I'd like to have some nested groups:

const toPath = pathToRegexp.compile("(/first/:one(/second/:two)?)?/three");

toPath(); // returns `/three`
toPath({ two : 2 }); // returns `/three` because outer group "first" is not matched
toPath({ one : 1 }); // returns `/first/1/three`
toPath({ one : 1, two: 2 }); // returns `/first/1/second/2/three`

Is this even possible using this library?

LKay avatar Mar 06 '18 01:03 LKay

No, it's not possible like that. You can't embed params inside a matching group, it's a regexp expected inside there. It could be possible to consider this in future releases, but right now the only way to do it is to create two separate paths and use an array or use a regexp.

blakeembrey avatar Mar 07 '18 05:03 blakeembrey

Another idea would be to introduce nested groups that support paths instead of regexps and may solve a couple of other issues people are struggling with also. Something like:

{/first/:one{/second/:two}}/three

The parts inside {} are paths instead of regexps. To support this, however, we'd need to solve the same problem as https://github.com/pillarjs/path-to-regexp/issues/95 which will likely entail rewriting the parser to be an actual parser instead of regexps.

blakeembrey avatar Mar 07 '18 06:03 blakeembrey

Fair enough, thanks for replying.

Yeah, introducing {} for defining groups is a nice idea!

First extract groups then perform usual regex conversion to whatever is inside {} and finally compile it all together by flattening extracted groups from the most nested one.

LKay avatar Mar 07 '18 07:03 LKay

Actually with version 0.1.7 this has worked. Check here: http://forbeslindesay.github.io/express-route-tester/

delijah avatar Mar 29 '18 08:03 delijah

@delijah I'm not sure anyone is arguing against that, but 0.1.x had a host of other bugs that resulted from being able to write paths like this. It was never an official feature, there was a lot of things you could do with it because it was extremely lax.

blakeembrey avatar Mar 30 '18 00:03 blakeembrey

Any progress on this?

delijah avatar Dec 19 '18 10:12 delijah

@delijah The best way to get progress on this is to submit a PR. I don't think the feature is particularly clear on possible edge cases right now (probably clearer once someone implements it) and it will need enough test coverage to ensure it works correctly.

blakeembrey avatar Nov 11 '19 03:11 blakeembrey

I tried implementing this, but one unclear feature is how compile works - from this path, how would we get a valid string back? What is the expected output?

blakeembrey avatar Nov 12 '19 00:11 blakeembrey

@blakeembrey For the path (/first/:one)?/:second this is the expected output {second: 'foo'} => /foo {one: 'bar', second: 'foo'} => /first/bar/foo

Tofandel avatar Jul 30 '20 17:07 Tofandel

@Tofandel That example is the really only the tip of the iceberg, what about {/first/:one?}? or other nested groupings? If one is missing, does the child still get compiled? The proposal needs a solution for both parsing and compiling. If someone wants to put together such a proposal complete with edge cases, I can review implementing it.

I think I can make an attempt now that we refactored the parser though, and for now make the assumption OP does - if the first isn’t filled then nested gets ignored.

blakeembrey avatar Jul 30 '20 18:07 blakeembrey

{/first/:one?}? is equivalent to (/first)?/:one? so it would compile to this (/first/:one?)?/:second {second: 'foo'} => compiles to /foo => also parses /first/foo {one: 'bar', second: 'foo'} => /first/bar/foo

Regarding nesting it would work as you'd expect

(/first/:one(/nested/:nested))?/:second {one: 'bar', second: 'foo'} => /foo //nested is required so first part doesn't match {one: 'bar', nested: 'baz', second: 'foo'} => /first/bar/nested/baz/foo

(/first/:one(/nested/:nested)?)?/:second {one: 'bar', second: 'foo'} => /first/bar/foo //nested is optional so first part matches

That's really all the edge cases I see

Tofandel avatar Jul 30 '20 18:07 Tofandel

Cool, feel free to submit a PR! Maybe some more will come up when you try implementing it, I recall running into issues when I actually tried to code it but that was also over two years back now.

blakeembrey avatar Jul 30 '20 19:07 blakeembrey

const toPath1 = pathToRegexp.compile("/three");
const toPath2 = pathToRegexp.compile("/first/:one/three");
const toPath3 = pathToRegexp.compile("/first/:one/second/:two/three");

function toPathGroup() {
  const pathObject = { ...args };
  if (Object.hasOwnProperty("two")) return toPath3(pathObject);
  if (Object.hasOwnProperty("one")) return toPath2(pathObject);
  return toPath1(pathObject);
}

are you need toPathGroup?

idler8 avatar Feb 08 '23 02:02 idler8

No, I don't need it

idler8 avatar Feb 08 '23 02:02 idler8