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

Bug: Stateful regex problem #5

Closed
Xiphe opened this issue Feb 10, 2021 · 5 comments
Closed

Bug: Stateful regex problem #5

Xiphe opened this issue Feb 10, 2021 · 5 comments
Assignees

Comments

@Xiphe
Copy link

Xiphe commented Feb 10, 2021

When I use a regex with global flag, I get unexpected results since rgx.test is used to test against the asset path.

See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test#using_test_on_a_regex_with_the_global_flag for details on this.

Solution would be to use Boolean(asset.match((pattern as RegExp)) here

I'll happily provide a PR for this.

@swimmadude66
Copy link
Owner

I appreciate the call-out! I will evaluate this tonight, but I am curious: What benefit is there to passing a global regex as the pattern? Would it cause any issues with your usecase if we simply force the global flag to false before running the regex test?

@swimmadude66 swimmadude66 self-assigned this Feb 12, 2021
@swimmadude66
Copy link
Owner

@Xiphe
Copy link
Author

Xiphe commented Feb 13, 2021

Cool! Thanks for fixing this.
Regarding the use-case: In my case I could probably just omit the global flag and I cant really think of a good situation where this would enable something.

The benefit of having this fixed is more about predictability, I guess.
I was just irritated why the plugin behaved this way and then needed to debug until I landed here. So I assume the fix will at least save others some time that just blindly add a global flag to their regexp :)

@swimmadude66
Copy link
Owner

Absolutely a good point. I ended up using .match as you suggested anyways, but I wanted to make sure I was covering all bases. Thanks again for bringing this up, I never knew about that quirk of regexp.test!

@Xiphe
Copy link
Author

Xiphe commented Feb 15, 2021

I also forget about that all the time :) Thanks again 👍

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

No branches or pull requests

2 participants