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

fix: enhance req.acceptsCharsets method #6088

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Abdel-Monaam-Aouini
Copy link

@Abdel-Monaam-Aouini Abdel-Monaam-Aouini commented Oct 27, 2024

Refactor acceptsCharsets to support flexible charset input formats

This change improves the acceptsCharsets method to handle multiple input formats:

  • Single charset string (e.g., "utf-8")
  • Multiple charset arguments (e.g., "utf-8", "iso-8859-1")
  • Comma-delimited charset lists (e.g., "utf-8, iso-8859-1")

Updates include:

  • Enhanced documentation with clear examples and parameter descriptions
  • Modernized code using const instead of var
  • Improved function signature to use rest parameters (...charsets)
  • Clarified behavior regarding Accept-Charset header matching
  • Updated return value documentation to specify array return possibility

The changes make the API more flexible while maintaining backwards compatibility.

@UlisesGascon
Copy link
Member

Hey @Abdel-Monaam-Aouini! Thanks for this PR... can you explain the motivation for this change?

@Abdel-Monaam-Aouini
Copy link
Author

Hey @Abdel-Monaam-Aouini! Thanks for this PR... can you explain the motivation for this change?

The motivation for this change is to improve developer experience and make the acceptsCharsets API more flexible and intuitive. Currently, the method's behavior and supported input formats aren't clearly documented, which can lead to confusion.

Key motivations:

  1. Support Multiple Input Formats:

    • Developers should be able to check charset acceptance using the most convenient format for their use case
    • Some might prefer passing multiple arguments: req.acceptsCharsets('utf-8', 'iso-8859-1')
    • Others might work with comma-delimited strings: req.acceptsCharsets('utf-8, iso-8859-1')
    • The current documentation doesn't make these options clear
  2. Modern JavaScript Practices:

    • Updated the implementation to use modern ES6+ features like const and rest parameters
    • This makes the code more maintainable and consistent with modern JavaScript standards
  3. Better Documentation:

    • Added clear examples showing different usage patterns
    • Improved parameter and return value documentation
    • Made it explicit how the method interacts with the Accept-Charset header

The changes don't break existing functionality but make the API more developer-friendly and better documented. This helps reduce potential confusion and makes the feature more accessible to new Express.js users.

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.

2 participants