Skip to content

fix: replace deprecated trimRight() with trimEnd()#7265

Open
tejgokani wants to merge 1 commit into
expressjs:masterfrom
tejgokani:fix/trimright-to-trimend
Open

fix: replace deprecated trimRight() with trimEnd()#7265
tejgokani wants to merge 1 commit into
expressjs:masterfrom
tejgokani:fix/trimright-to-trimend

Conversation

@tejgokani
Copy link
Copy Markdown

Description

The host getter in lib/request.js:427 uses the deprecated trimRight() method, which is an Annex B legacy alias for trimEnd().

Why This Matters

  • trimRight() is deprecated and may emit warnings in future Node.js/V8 versions
  • trimEnd() is the ES2019 standard equivalent with identical behavior
  • This aligns with Express's modernization efforts

Changes

  • Replaced .trimRight() with .trimEnd() in the host getter

Testing

  • Existing tests pass (run npm test)
  • No new tests needed (this is a 1:1 method replacement with identical behavior)

Closes #XXXX (only if linking to an existing issue)

Copy link
Copy Markdown
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

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

trimEnd was added in Node.js 10.0.0, so this should be safe 👍

(trimRight have been available since at least Node.js 0.10)

@tejgokani
Copy link
Copy Markdown
Author

okay so am i approved to merge?

@LinusU
Copy link
Copy Markdown
Member

LinusU commented May 19, 2026

CI is green, but will wait for one other maintainer to look at it as well 🚀

@tejgokani
Copy link
Copy Markdown
Author

tejgokani commented May 19, 2026 via email

@tejgokani
Copy link
Copy Markdown
Author

All changes are approved, so will it get merged, or there's anything to be done from my side.
@LinusU

@krzysdz
Copy link
Copy Markdown
Contributor

krzysdz commented May 19, 2026

There is also #7047, which replaces .trimRight(), but also for some reason adds .split(). I think I planned asking the author why did they introduce .split(), but for some reason I did not do it.

If this is merged, then #7047 probably should be closed.

@tejgokani
Copy link
Copy Markdown
Author

Great! I appreciate the consideration. I'd like to keep moving forward with my PR.

My focus is on a clean, focused fix: replacing the deprecated trimRight() with
trimEnd(). This is a straightforward modernization with minimal risk.

I noticed #7047 also addresses this but adds .split() which seems like a separate
concern. Once my PR merges, I'll personally investigate why .split() was added
and follow up with a separate issue/PR if needed.

This keeps the changes focused and reviewable: one fix per PR. I'm committed to
seeing this through and ensuring the .split() question gets answered.

Let me know if you'd like me to make any adjustments! 🙌

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.

3 participants