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

Add back keys property to regex #272

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

Conversation

nickyu42
Copy link

@nickyu42 nickyu42 commented Mar 8, 2022

See issue #236

The returned regex object from pathToRegex used to include the modified
keys array as a keys property in v1.7.0. The current API is not entirely
ideal, since a keys array must be passed, we could instead use a more
functional API without mutation.

In the future this should be changed that no array needs to be passed to pathToRegex,
adding it as a property this way is done to maintain backwards compatibility.

The returned regex object from `pathToRegex` used to include the modified
keys array as a `keys` property in v1.7.0. The current API is not entirely
ideal, since a keys array must be passed, we could instead use a more
functional API where no array is mutated.

In the future this should be changed that no array needs to be passed to
`pathToRegex`, adding it as a property is done for backwards compatibility.
/**
* Optional property that includes key metadata corresponding to this regex.
*/
keys?: Key[];
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this required? A way to get around TypeScript complaining is to use Object.assign(regex, { keys }).

@blakeembrey
Copy link
Member

We'll also need a simple couple of tests to ensure we don't regress but otherwise LGTM 😄

@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #272 (845c7fb) into master (762bc6b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #272   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          246       250    +4     
  Branches        98        98           
=========================================
+ Hits           246       250    +4     
Impacted Files Coverage Δ
src/index.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 762bc6b...845c7fb. Read the comment docs.

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

Successfully merging this pull request may close these issues.

None yet

3 participants