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

fix(no-large-snapshots): actually compare allowed name strings to name #625

Merged
merged 1 commit into from
Jul 26, 2020

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Jul 12, 2020

Turns out when I converted this rule to TypeScript, I dropped the actual comparator, which apparently no ones noticed 😅

Came across this while doing #624, as I realised we're accepting RegExp instances from .eslintrc.js which isn't strictly wrong, but I think it might be worth deprecating in favor of just always taking RegExp strings as that's easy and reduces the chance of being broken by a minor version of ESLint.

The migration would be that people just add ^ & $ to the start and end of their strings respectively, and then add .source to their RegExps.

Edit: actually because we're accepting RegExp instances, we're getting both the source and flags, so we could do this change without it being breaking as we'd just wrap the strings in new RegExp(^${name}$`).

That means the breaking change would be no longer allowing devs to set RegExp flags used in matching, which shouldn't cause too much trouble as i would be the main useful flag, which you can implement long hand 🤔

I mention this here as technically the behaviour is broken already, so it could be argued to be a breaking bugfix?

@G-Rath G-Rath requested a review from SimenB July 12, 2020 03:35
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Bugfix, agreed. If it breaks somebody, we can fix then

@G-Rath G-Rath merged commit 622a08c into master Jul 26, 2020
@G-Rath G-Rath deleted the fix-bug-in-no-large-snapshots branch July 26, 2020 05:39
github-actions bot pushed a commit that referenced this pull request Jul 26, 2020
## [23.18.1](v23.18.0...v23.18.1) (2020-07-26)

### Bug Fixes

* **no-large-snapshots:** actually compare allowed name strings to name ([#625](#625)) ([622a08c](622a08c))
@github-actions
Copy link

🎉 This PR is included in version 23.18.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants