Skip to content

docs: fix JSDoc for req.accepts() return value and parameter format#6936

Merged
bjohansebas merged 6 commits intoexpressjs:masterfrom
marcosmol204:update-request-accepts-js-doc
Jan 16, 2026
Merged

docs: fix JSDoc for req.accepts() return value and parameter format#6936
bjohansebas merged 6 commits intoexpressjs:masterfrom
marcosmol204:update-request-accepts-js-doc

Conversation

@marcosmol204
Copy link
Copy Markdown
Contributor

Description

This pull request updates the JSDoc for req.accepts() to correct two inaccuracies in the current documentation. These updates align the JSDoc with the behavior of the underlying npm package used by Express for content negotiation.

What This PR Fixes

  1. Incorrect return value
    The existing JSDoc states that the method returns undefined when no acceptable type is found.
    In reality, the underlying negotiation package returns false, and that is what Express exposes.

    Supporting code (from the underlying accepts package):

    // returns the best match or false
    return type || false
  2. Comma-separated values are not supported
    Some examples suggested that a comma-separated string (e.g., "json, text") is treated as multiple types.
    The npm package does not parse comma-separated strings.
    Only arrays or multiple arguments are supported.

    Supporting code (from the same package):

     if (!Array.isArray(types)) {
       types = new Array(arguments.length);
       for (var i = 0; i < types.length; i++) {
         types[i] = arguments[i];
       }
     }

This shows that each argument is captured individually, and values are not split on commas.

Why This Matters

The old JSDoc can mislead developers when debugging Accept header behavior or relying on inline IntelliSense.
This PR corrects the documentation so it accurately reflects the behavior of the npm module Express depends on.

Disclaimer

The incorrect assumptions present in the old JSDoc also exist in the underlying npm package.
This PR updates only the Express documentation and does not modify or fix the dependency itself.

Copy link
Copy Markdown

@Hardanish-Singh Hardanish-Singh left a comment

Choose a reason for hiding this comment

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

LGTM

@krzysdz
Copy link
Copy Markdown
Contributor

krzysdz commented Jan 6, 2026

Could you also update the return value here? It should be false, not undefined.

express/lib/request.js

Lines 112 to 115 in 2d0884f

* // Accept: text/*, application/json
* req.accepts('image/png');
* req.accepts('png');
* // => undefined

@krzysdz
Copy link
Copy Markdown
Contributor

krzysdz commented Jan 6, 2026

The example with 'json, text' should be updated to use argument list 'json', 'text' like in the docs of accepts.

express/lib/request.js

Lines 107 to 108 in 2d0884f

* req.accepts('json, text');
* // => "json"

@marcosmol204
Copy link
Copy Markdown
Contributor Author

@krzysdz Done :)

Copy link
Copy Markdown
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread lib/request.js
* req.accepts('text/html');
* // => "text/html"
* req.accepts('json, text');
* req.accepts('json', 'text');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was looking for how long we haven't had that support; it happened in cec0c06, for https://github.com/expressjs/express/releases/tag/4.0.0-rc1.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That is, that support didn’t come when the accepts package was created.

@bjohansebas bjohansebas changed the title fixed request accept jsdoc docs: fix JSDoc for req.accepts() return value and parameter format Jan 16, 2026
@bjohansebas bjohansebas linked an issue Jan 16, 2026 that may be closed by this pull request
@bjohansebas bjohansebas merged commit ae265a9 into expressjs:master Jan 16, 2026
28 checks passed
krzysdz added a commit to krzysdz/accepts that referenced this pull request Jan 18, 2026
krzysdz added a commit to krzysdz/accepts that referenced this pull request Jan 18, 2026
Port changes from expressjs/express#6936 and expressjs/express#2527.

Co-Authored-By: Marcos Molina <53741892+marcosmol204@users.noreply.github.com>
Co-Authored-By: Elvin Yung <elvin@elvinyung.com>
jonchurch pushed a commit to krzysdz/accepts that referenced this pull request Jan 20, 2026
Port changes from expressjs/express#6936 and expressjs/express#2527.

Co-Authored-By: Marcos Molina <53741892+marcosmol204@users.noreply.github.com>
Co-Authored-By: Elvin Yung <elvin@elvinyung.com>
jonchurch pushed a commit to jshttp/accepts that referenced this pull request Jan 20, 2026
Port changes from expressjs/express#6936 and expressjs/express#2527.

Co-authored-by: krzysdz <krzysdz@users.noreply.github.com>
Co-authored-by: Marcos Molina <53741892+marcosmol204@users.noreply.github.com>
Co-authored-by: Elvin Yung <elvin@elvinyung.com>
kylewilliams60 added a commit to kylewilliams60/https-accepts that referenced this pull request Feb 11, 2026
Port changes from expressjs/express#6936 and expressjs/express#2527.

Co-authored-by: krzysdz <krzysdz@users.noreply.github.com>
Co-authored-by: Marcos Molina <53741892+marcosmol204@users.noreply.github.com>
Co-authored-by: Elvin Yung <elvin@elvinyung.com>
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.

Request.accepts documentation

4 participants