From fd29ee34d6a14e7dda824e7cd3b3ed6ae3cbfc61 Mon Sep 17 00:00:00 2001
From: Paul Melnikow <email@paulmelnikow.com>
Date: Tue, 4 Dec 2018 19:05:54 -0500
Subject: [PATCH] Fix handling of optionals in the middle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Thanks so much for this library! It’s come in very handy in Shields. We’ve significantly reduced the duplication in our route definitions, and we’re getting really close letting developers build badge URLs in the frontend.

We’re running into an issue when an optional token precedes the extension. Many of the routes look something like this:

`/travis/:user/:repo/:branch?.ext(svg|png)`

- That pattern matches `/travis/foo/bar/master.svg` as we’d expect
- Unexpectedly it _does not_ match `/travis/foo/bar.svg`
- Unexpectedly it _does_ match `/travis/foo/bar/.svg`

I traced the behavior back to #71 where it was introduced. I _think_ it was unintentional since that PR was about making the leading delimiter optional for an optional token. I don’t see why the delimiter should be present before an optional in the middle.

It’s kind of an odd fix, though it makes some sense given #71 was about fixing leading optionals.

@RedSparr0w was asking about paths like `/:optional?/:required.json` so
I added another test example to the test for that.
---
 index.js |  4 ++--
 test.ts  | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/index.js b/index.js
index d6c45f9..1f3d982 100644
--- a/index.js
+++ b/index.js
@@ -186,7 +186,7 @@ function tokensToFunction (tokens) {
 
       if (token.optional) {
         // Prepend partial segment prefixes.
-        if (token.partial) path += token.prefix
+        if (i === 0 && token.partial) path += token.prefix
 
         continue
       }
@@ -323,7 +323,7 @@ function tokensToRegExp (tokens, keys, options) {
       if (keys) keys.push(token)
 
       if (token.optional) {
-        if (token.partial) {
+        if (i === 0 && token.partial) {
           route += escapeString(token.prefix) + '(' + capture + ')?'
         } else {
           route += '(?:' + escapeString(token.prefix) + '(' + capture + '))?'
diff --git a/test.ts b/test.ts
index 6cee055..0be4097 100644
--- a/test.ts
+++ b/test.ts
@@ -827,9 +827,11 @@ var TESTS: Test[] = [
       '/bar'
     ],
     [
+      ['/bar', ['/bar', undefined]],
       ['/foo/bar', ['/foo/bar', 'foo']]
     ],
     [
+      [null, '/bar'],
       [{ test: 'foo' }, '/foo/bar']
     ]
   ],
@@ -2291,6 +2293,40 @@ var TESTS: Test[] = [
       [{ postType: 'random' }, null]
     ]
   ],
+  [
+    '/:required/:optional?-ext',
+    null,
+    [
+      {
+        name: 'required',
+        prefix: '/',
+        delimiter: '/',
+        optional: false,
+        repeat: false,
+        partial: false,
+        pattern: '[^\\/]+?'
+      },
+      {
+        name: 'optional',
+        prefix: '/',
+        delimiter: '/',
+        optional: true,
+        repeat: false,
+        partial: true,
+        pattern: '[^\\/]+?'
+      },
+      '-ext'
+    ],
+    [
+      ['/foo-ext', ['/foo-ext', 'foo', undefined]],
+      ['/foo/bar-ext', ['/foo/bar-ext', 'foo', 'bar']],
+      ['/foo/-ext', null]
+    ],
+    [
+      [{ required: 'foo' }, '/foo-ext'],
+      [{ required: 'foo', optional: 'baz' }, '/foo/baz-ext']
+    ]
+  ],
 
   /**
    * Unicode characters.