Skip to content

Make controller compatible with express versions 4 and 5#213

Open
ushkarev wants to merge 1 commit intoHMPO:masterfrom
ministryofjustice:master
Open

Make controller compatible with express versions 4 and 5#213
ushkarev wants to merge 1 commit intoHMPO:masterfrom
ministryofjustice:master

Conversation

@ushkarev
Copy link

@ushkarev ushkarev commented Jun 25, 2025

  • express 4’s router methods allow wildcard paths in the form '*'
  • express 5’s router methods require wildcard placeholders in paths to be named, eg. '*allPaths'
  • …and there is no wildcard syntax that is common to both. For example, express 4 treats '*allPaths' as requiring the text “allPaths” to appear in the URL path.

However, both allow specifying a RegExp object directly, thereby bypassing the use of differing versions of path-to-regexp (which are used under the hood on string paths).

This should address #208 but by different means.

- `express` 4’s router methods allow wildcard paths in the form `'*'`
- `express` 5’s router methods require wildcard placeholders in paths to be named, eg. `'*allPaths'`
- …and there is no wildcard syntax that is common to both. For example, `express` 4 treats `'*allPaths'` as requiring the text “allPaths” to appear in the URL path.

However, both allow specifying a `RegExp` object directly, thereby bypassing the use of differing versions of `path-to-regexp` (which are used under the hood on string paths).
@ushkarev
Copy link
Author

ushkarev commented Jun 25, 2025

I ran the tests locally against 4 and 5 and they both seem happy. I didn’t want to mess with your workflows so didn’t add anything to ci.yaml for testing against both versions.

What do you think about this approach? Alternatives I considered:

  • discover the current express version and use different string constants (doesn’t work since version is not exposed anywhere in the express lib and reading package.json feels dirty)
  • an express-5 branch but that would be a bit too messy and forking would lead to divergence
  • trying '*' first, catching the appropriate error from path-to-regexp then retrying '*allPaths' (I figure this is a bit brittle, could catch the wrong error, could be a bit slow)

@ushkarev
Copy link
Author

ushkarev commented Jul 7, 2025

@cmcgee4801995 I notice you’ve made some recent commits to this repo… what do you think about this PR?

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.

1 participant