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(express): Support Express v5 #4201

Merged
merged 3 commits into from
Sep 23, 2024
Merged

Conversation

wobsoriano
Copy link
Member

Description

This PR sets a minimum Express 4 version and support for Express 5.

For Express ^4.17.0 - we went with this version as it's a more stable version, to avoid issues with earlier 4.x.x releases. We didn't want to start all the way back at 4.0.0 and risk running into old bugs or outdated features.

Here's the total downloads of 4.17.x:

Screenshot 2024-09-20 at 8 56 36 AM

For v5, there is not really a big breaking change and it aims to support backward compatibility with Express 4. Here's a small breaking note that should not affect our SDK. Full changelog can be found here if interested.

Note that Express v5 is still tagged as next and not latest even though it's stable. But we want to add support to it now as we don't want to release another major change in the future.

I tested v5 locally with our current CI test and it passed.

This is part of making our Express SDK ready.

Resolves ECO-192

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Copy link

changeset-bot bot commented Sep 20, 2024

🦋 Changeset detected

Latest commit: 83c09a2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clerk/express Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -68,6 +68,9 @@
"tsup": "*",
"typescript": "*"
},
"peerDependencies": {
"express": "^4.17.0 || ^5.0.0"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we support v5 out-of-the-box actually but we want to set a more stable minimum v4 version to prevent any breaking change in earlier versions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clerk/express is still v0. Are we doing v1 after this PR, or we wait for Express 5 to be cut as latest and then do v1 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets do v1 now unless we know that express v5 is coming out soon

Copy link
Member Author

@wobsoriano wobsoriano Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, here's the pending items before they tag v5 as latest:

Screenshot 2024-09-23 at 7 52 45 AM

Mostly docs update, so I think latest tag will come soon. Let's go v1 major? v5 is backwards compatible anyway...

@@ -0,0 +1,5 @@
---
"@clerk/express": patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think minor is more appropriate

Suggested change
"@clerk/express": patch
"@clerk/express": minor

Copy link
Member Author

@wobsoriano wobsoriano Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to major since Express v5 tagged as latest will come sooner than later anyway and that we can start deprecating @clerk/clerk-sdk-node too (migration PR docs already in review). What y'all think?

cc @nikosdouvlis

Copy link
Member

@nikosdouvlis nikosdouvlis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -68,6 +68,9 @@
"tsup": "*",
"typescript": "*"
},
"peerDependencies": {
"express": "^4.17.0 || ^5.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets do v1 now unless we know that express v5 is coming out soon

@wobsoriano wobsoriano merged commit 99de68d into main Sep 23, 2024
20 checks passed
@wobsoriano wobsoriano deleted the rob/eco-192-express-v5-compat branch September 23, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants