Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(matchMock): handle regex mock paths directly #281

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

smackfu
Copy link
Member

@smackfu smackfu commented Nov 27, 2024

Description

Handle regex paths directly rather than passing them to path-to-regexp.

Motivation and Context

#269 upgraded path-to-regexp to v8 from v3. An error was reported when using a regexp for the path variable with that change.

Scenario:

const scenarios = {
  test: [
    {
      request: {
        method: 'GET',
        path: /^\/api\/v1\/metadata\/variables\/draft\/\w+$/,
      },
      response: {},
    },
  ],
};

Error after doing any GET call to the parrot mock:

FATAL: str is not iterable
    reason: "unhandledRejection"
    error: {
      "type": "TypeError",
      "message": "str is not iterable",
      "stack":
          TypeError: str is not iterable
              at lexer (/opt/module-workspace/foo/node_modules/parrot-core/node_modules/path-to-regexp/dist/index.js:43:23)
              at lexer.next (<anonymous>)

Looking at the path-to-regexp README shows a missed signature change:

v3 README:

path A string, array of strings, or a regular expression.

v8 README:

path String or array of strings.

I'm not 100% clear on why passing in a regex is failing, and it still seems to work in unit tests. But it seems wise to not do it since it's undocumented. We can just check for a regex path directly and evaluate it without path-to-regexp at all.

How Has This Been Tested?

Added new unit tests. Verified this fixes issue in cited example.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • My changes are in sync with the code style of this project.
  • There aren't any other open Pull Requests for the same issue/update.
  • These changes should be applied to a maintenance branch.
  • This change requires cross browser checks.
  • This change impacts caching for client browsers.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using parrot?

Fixes issue with regexp paths.

@smackfu smackfu requested a review from a team as a code owner November 27, 2024 17:43
@bishnubista bishnubista merged commit d92d648 into main Dec 3, 2024
6 checks passed
@bishnubista bishnubista deleted the handle-regex-path branch December 3, 2024 05:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants