-
Notifications
You must be signed in to change notification settings - Fork 214
Rename POST_RECEIVE_V2 to POST_RECEIVE #724
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
Conversation
🦋 Changeset detectedLatest commit: a885841 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
WalkthroughThis pull request introduces a major version update for the Changes
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
It would be nice to use a different event name for all who are still using deprecated event name. Or is it OK for user to silently transition to use new event? |
I think the best way forward is to rename. Otherwise it means removing two events and introducing a new one. |
* main: Refactor server to run in all (Node.js compatible) runtimes and meta frameworks (#710) [ci] release (#721) @tus/s3-store: fix unhandled promise rejection (#728) Bump @aws-sdk/client-s3 from 3.717.0 to 3.758.0 (#733) Bump @types/node from 22.10.1 to 22.13.7 (#734) Bump typescript from 5.6.2 to 5.8.2 (#731) @tus/azure-store: fix non-ASCII characters error on metadata (#725) Fix package-lock.json @tus/s3-store: add missing docs for `maxMultipartParts` (#720) [ci] release (#719) @tus/server: don't use AbortSignal.any to fix memory leak (#718) [ci] release (#713) Add .coderabbit.yml @tus/s3-store: add `maxMultipartParts` option (#712) [ci] release (#701) @tus/s3-store: add `minPartSize` option (#703)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/README.md (1)
82-86
: Update the Event Reference in the Options DocumentationThe description for
options.postReceiveInterval
still points toPOST_RECEIVE_V2
, whereas this PR renames that event toPOST_RECEIVE
. Updating the link reference and text here will ensure consistency in the documentation.-Interval in milliseconds for sending progress of an upload over [`POST_RECEIVE_V2`](#eventspost_receive_v2) (`number`). +Interval in milliseconds for sending progress of an upload over [`POST_RECEIVE`](#eventspost_receive) (`number`).🧰 Tools
🪛 LanguageTool
[uncategorized] ~84-~84: A punctuation mark might be missing here.
Context: ... for sending progress of an upload over [POST_RECEIVE_V2
](#eventspost_receive_v...(AI_EN_LECTOR_MISSING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
85-85: Link fragments should be valid
null(MD051, link-fragments)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (9)
.changeset/spicy-garlics-look.md
(1 hunks)packages/server/README.md
(1 hunks)packages/server/src/handlers/BaseHandler.ts
(1 hunks)packages/server/src/handlers/PatchHandler.ts
(0 hunks)packages/server/src/server.ts
(0 hunks)packages/server/src/types.ts
(1 hunks)packages/server/test/PatchHandler.test.ts
(1 hunks)packages/server/test/Server.test.ts
(1 hunks)packages/utils/src/constants.ts
(0 hunks)
💤 Files with no reviewable changes (3)
- packages/server/src/server.ts
- packages/utils/src/constants.ts
- packages/server/src/handlers/PatchHandler.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/server/test/Server.test.ts (1)
packages/utils/src/constants.ts (1)
EVENTS
(106-111)
🔇 Additional comments (6)
packages/server/src/types.ts (1)
43-46
: Documentation Update forpostReceiveInterval
The documentation comment has been updated to referenceEVENTS.POST_RECEIVE
instead of the deprecatedEVENTS.POST_RECEIVE_V2
. This change accurately reflects the renamed event per the PR objectives and helps maintain consistency between the documentation and event handling in the codebase..changeset/spicy-garlics-look.md (1)
1-7
: LGTM! Version changes and deprecation notice properly documented.The changeset correctly documents the major version update to
@tus/server
and minor version update to@tus/utils
, along with the renaming ofPOST_RECEIVE_V2
toPOST_RECEIVE
and removal of the deprecated version.packages/server/src/handlers/BaseHandler.ts (1)
164-164
: Event name updated correctly per PR objectives.The event emission has been properly updated from
POST_RECEIVE_V2
toPOST_RECEIVE
, which aligns with the PR goal of standardizing the event naming and resolving the long-standing deprecation.packages/server/test/Server.test.ts (1)
460-460
: Test updated to use the renamed event.The event listener has been correctly updated to use
EVENTS.POST_RECEIVE
instead of the previousEVENTS.POST_RECEIVE_V2
, keeping the test aligned with the implementation changes.packages/server/test/PatchHandler.test.ts (1)
9-9
: Import statement updated after event emission removal.The removal of the
EVENTS
import from the import statement aligns with the broader changes in the PR where the deprecatedPOST_RECEIVE
event emission in the PATCH handler was removed. The test file no longer needs to verify this event emission.packages/server/README.md (1)
271-272
:✅ Verification successful
Correct the Callback Arrow Function Syntax in the Example
The updated code snippet correctly uses the new event constant (
POST_RECEIVE
); however, the arrow function syntax in the example appears to be incorrect. It should properly wrap both parameters in parentheses.- server.on(EVENTS.POST_RECEIVE, (req, upload => {}) + server.on(EVENTS.POST_RECEIVE, (req, upload) => {})
Fix Callback Arrow Function Parentheses Syntax
- The callback function for the
POST_RECEIVE
event is missing proper wrapping of its parameters.- Update the snippet to enclose both parameters in parentheses to comply with valid arrow function syntax.
- server.on(EVENTS.POST_RECEIVE, (req, upload => {}) + server.on(EVENTS.POST_RECEIVE, (req, upload) => {})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/server/README.md (2)
84-86
: Fix the link fragment for thePOST_RECEIVE
event.The current link
[#eventspost_receiveinterval]
does not appear to match any valid header in the document. Consider updating it to reference the correct anchor for thePOST_RECEIVE
event section—for example, using[#post_receive]
or the appropriate fragment generated for the header.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
85-85: Link fragments should be valid
null(MD051, link-fragments)
269-272
: Correct the arrow function syntax in the event listener snippet.The example code currently uses
(req, upload => {})
, which is syntactically incorrect. It should be updated to(req, upload) => {}
so that users copying the example won’t encounter issues.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/server/README.md
(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/server/README.md
85-85: Link fragments should be valid
null
(MD051, link-fragments)
Do not merge before #710
POST_RECEIVE
is called every time an upload finished writing to the store. This event is emitted whenever the request handling is completed (which is the same asonUploadFinish
, almost the same asPOST_FINISH
), whereas thePOST_RECEIVE_V2
event is emitted while the request is being handled.POST_RECEIVE
has been marked deprecated for a long time.