Skip to content

Make it clear what arguments req.accepts* functions accept - #7452

Draft
krzysdz wants to merge 1 commit into
expressjs:masterfrom
krzysdz:acceptsX-tests-and-docs
Draft

Make it clear what arguments req.accepts* functions accept#7452
krzysdz wants to merge 1 commit into
expressjs:masterfrom
krzysdz:acceptsX-tests-and-docs

Conversation

@krzysdz

@krzysdz krzysdz commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Background

#6088 changed the wording of req.acceptsCharsets() docs and introduced a mistake which went unnoticed until #7451, where it was reported as a bug in code. Interestingly, 9 days later #6936 fixed the same mistake in docs for req.accepts() and in #6936 (comment) @bjohansebas pointed out that Express had removed support for a comma delimited string before 4.0 release. Historically it looks more or less like this:

  1. (2014) Express 4 switches to jshhtp/accepts, which does not accept a single comma delimited string, but leaves it in the docs.
  2. (2024-10-27) fix: enhance req.acceptsCharsets method #6088 is opened and mirrors the wrong example from req.accepts() to req.acceptsCharsets(). The PR appears to be at least AI-assisted, if not completely AI-generated, but this does not explain why nobody noticed this during review. It is possible that the text wasn't based on req.accepts(), but the LLM just wrote false information in docs and PR description - it sounds as if the author completely rewrote the method to extend the functionality, while in reality the changes to code were just cosmetic.
  3. (2025-12-02) docs: fix JSDoc for req.accepts() return value and parameter format #6936 is opened with a partial fix to the req.accepts() docs.
  4. (2026-01-06) I noticed that docs: fix JSDoc for req.accepts() return value and parameter format #6936 missed some spots in the docs.
  5. (2026-01-07) fix: enhance req.acceptsCharsets method #6088 is merged - the wrong req.acceptsCharsets() docs become a part of Express.
  6. (2026-01-08) docs: fix JSDoc for req.accepts() return value and parameter format #6936 is updated to eliminate the rest of mistakes in req.accepts() docs.
  7. (2026-01-16) @bjohansebas finds the reason why there was a mistake in the first place and merges docs: fix JSDoc for req.accepts() return value and parameter format #6936.

Conclusions? Maybe I should check PRs after they're merged if I've ignored them before...

Description

This PR fixes this particular docs error and adds tests that cover the types of accepted arguments of req.accepts* functions.

Remaining things

All of these methods (req.accepts, req.acceptsEncodings, req.acceptsCharsets, req.acceptsLanguages) do the same thing (except calling different methods of accepts), but are written in 3 different ways:

  1. no args, temporary variable (var), .apply(accept, arguments)

    express/lib/request.js

    Lines 127 to 130 in 023767f

    req.accepts = function(){
    var accept = accepts(this);
    return accept.types.apply(accept, arguments);
    };

    express/lib/request.js

    Lines 140 to 143 in 023767f

    req.acceptsEncodings = function(){
    var accept = accepts(this);
    return accept.encodings.apply(accept, arguments);
    };
  2. rest parameters, temporary variable (const)

    express/lib/request.js

    Lines 171 to 174 in 023767f

    req.acceptsCharsets = function(...charsets) {
    const accept = accepts(this);
    return accept.charsets(...charsets);
    };
  3. rest parameters, everything chained

    express/lib/request.js

    Lines 185 to 187 in 023767f

    req.acceptsLanguages = function(...languages) {
    return accepts(this).languages(...languages);
    };

All of these functions are also documented completely differently, with or without examples and with JSDoc types not always reflecting the allowed argument types properly.

I'll try to make these functions and their docs more consistent with each other, so it'll be a draft PR for now.

TODO:

  • Make methods use same code style
  • Make the descriptions consistent and more detailed (especially *Encodings and *Languages)
  • Make sure that the types in JSDoc reflect reality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentations issues tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant