Skip to content

Conversation

Phillip9587
Copy link
Contributor

@Phillip9587 Phillip9587 commented Mar 26, 2025

This PR is a cleanup of the options handling in all 4 middleware functions. The changes primarily involve using optional chaining.

Follow up to #551

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the middleware parser options to simplify their use and improve readability by leveraging optional chaining. Key changes include:

  • Removing the fallback variable (opts) in favor of directly using the options parameter.
  • Updating the destructuring of options in normalizeOptions for urlencoded, json, raw, and text.
  • Adjusting the parameters forwarded to the read function to only include the essential ones.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
lib/types/urlencoded.js Simplified options handling and query parser creation using optional chaining, with adjustments in the read() arguments.
lib/types/json.js Streamlined extraction of options using optional chaining.
lib/types/raw.js Adopted consistent options handling by removing the fallback variable.
lib/types/text.js Uniform refactoring for options extraction and read() argument clarity.
Comments suppressed due to low confidence (2)

lib/types/urlencoded.js:38

  • Ensure that normalizeOptions correctly handles cases where options is undefined, as the fallback to an empty object (options || {}) has been removed.
var { inflate, limit, verify, shouldParse } = normalizeOptions(options, 'application/x-www-form-urlencoded')

lib/types/urlencoded.js:93

  • Verify that the removal of charsetSentinel and interpretNumericEntities from the read() call is intentional and does not affect the expected parser behavior.
read(req, res, next, parse, debug, { inflate, limit, verify })

@UlisesGascon
Copy link
Member

UlisesGascon commented Mar 26, 2025

I will merge this one, to be able to include it in the Release proposal @wesleytodd.

@UlisesGascon UlisesGascon merged commit 4d85c4c into expressjs:master Mar 26, 2025
12 checks passed
@UlisesGascon UlisesGascon mentioned this pull request Mar 26, 2025
@Phillip9587 Phillip9587 deleted the cleanup-after-normalize-options branch March 27, 2025 00:11
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