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

Ensure compatibility with latest versions of Express #96

Merged
merged 2 commits into from
Nov 20, 2024

Conversation

nwalters512
Copy link
Contributor

The most recent versions of Express (4.20.0 and above) changed the way that route patterns are compiled to regular expressions. This PR introduces compatibility with these new versions, while retaining compatibility with older versions. To prove this to myself, I updated the test suite to run against both old and new versions of Express. I recommend reviewing these changes with whitespace changes hidden so that you can see what actually changed in the tests.

@timd73
Copy link

timd73 commented Sep 16, 2024

@nwalters512 when will this be merged an a new version released?

@nwalters512
Copy link
Contributor Author

@timd73 I don't know; I don't maintain this library. I'm just someone who uses it and decided to file this PR to fix the compatibility issues I was experiencing in my own project. You'll have to ask @AlbertoFdzM.

@timd73
Copy link

timd73 commented Sep 16, 2024

@AlbertoFdzM ?

@timd73
Copy link

timd73 commented Sep 22, 2024

@nwalters512 so what are we thinking? Has @AlbertoFdzM ceased maintaining this library?

@nwalters512
Copy link
Contributor Author

I won't make assumptions about the maintainer or when he may be able to review this. I'm leaning towards forking this package and will likely do so if I don't hear anything by the end of this week. I hate to do this since there are a variety of other packages that rely on express-list-endpoints.

@nwalters512
Copy link
Contributor Author

nwalters512 commented Oct 9, 2024

I ultimately forked this package, applied the changes from this PR, and released it as @prairielearn/express-list-endpoints. It's an ESM-only package and only officially supports express@^4.21.0.

I'm leaving this PR open in the sincere hope that the maintainer is ultimately able to merge it, as I don't think having a fork is the best long-term solution for the ecosystem.

@btiernay
Copy link

@AlbertoFdzM Is there any chance you might be able to review and merge this request? It's quite fundamental. Really appreciate this project and your help here! 🙇

Signed-off-by: Alberto Fernandez <[email protected]>
@AlbertoFdzM AlbertoFdzM merged commit 49263c2 into AlbertoFdzM:main Nov 20, 2024
3 checks passed
@nwalters512 nwalters512 deleted the new-express-compat branch November 20, 2024 20:42
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.

4 participants