Skip to content
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

feat: add legacyRoutes option #223

Merged
merged 1 commit into from
May 15, 2024
Merged

feat: add legacyRoutes option #223

merged 1 commit into from
May 15, 2024

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented May 13, 2024

This adds the legacyRoutes option, defaulted to true.

When legacyRoutes is true, the ? character will automatically be escaped:

server.respondWith('GET', '/hello?world', handler);

When it is false (future default), ? has a special meaning in that it denotes which parameters in a path are optional. Due to this, it will not be escaped automatically:

server.respondWith('GET', '/hello\\?world', handler);

// so we can have optional params
server.respondWith('GET', '/hello/:param?', handler);

something like this @fatso83 maybe

though i do wonder if we could do some smarts here and auto-escape all ? except those following a param 🤔

This adds the `legacyRoutes` option, defaulted to `true`.

When `legacyRoutes` is true, the `?` character will automatically be
escaped:

```ts
server.respondWith('GET', '/hello?world', handler);
```

When it is false (future default), `?` has a special meaning in that it
denotes which parameters in a path are optional. Due to this, it will
not be escaped automatically:

```ts
server.respondWith('GET', '/hello\\?world', handler);

// so we can have optional params
server.respondWith('GET', '/hello/:param?', handler);
```
@fatso83
Copy link
Contributor

fatso83 commented May 15, 2024

Perfect. Thank you.

though i do wonder if we could do some smarts here and auto-escape all ? except those following a param 🤔

I don't see how, but this is fine as is.

@fatso83 fatso83 merged commit da6f09c into sinonjs:main May 15, 2024
@43081j 43081j deleted the legacy-routes branch May 15, 2024 11:41
@fatso83
Copy link
Contributor

fatso83 commented Sep 13, 2024

This might change things: #226

fatso83 added a commit that referenced this pull request Sep 13, 2024
This reverts commit da6f09c.

After the version 8 breaking changes in path-to-regexp this is
essentially a no-op, so this is not a breaking change.

closes #224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants