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 multiple usages of regexp-exec #919

Closed
wants to merge 1 commit into from

Conversation

Khartir
Copy link

@Khartir Khartir commented Mar 19, 2021

Closes #751

I would have liked to add a test for this, but I have honestly no idea of how to do so.

@zloirock
Copy link
Owner

I need some time to explore this issue deeper.

@zloirock
Copy link
Owner

zloirock commented Apr 7, 2021

core-js should be maximally modular and es.regexp.exec module should work without es.regexp.constructor or es.regexp.sticky.

@zloirock
Copy link
Owner

zloirock commented Apr 7, 2021

I'm confused in this code. @nicolo-ribaudo could you say something about #751, #754, #810?

@Khartir
Copy link
Author

Khartir commented Apr 7, 2021

@zloirock As far as I can tell, the problem with making es.regexp.exec, es.regexp.constructor and es.regexp.sticky independent of each other is that in order to support the y flag all have to be modified: the constructor to accept the flag, sticky to add the property and exec to actually handle it.

So maybe the solution should be to combine the modules into one? The only way to split this cleanly in different packages would probably be to have a sticky package that would modify constructor, property and exec all on its own and have the other packages only do what they did before. That would require overwriting the constructor and exec multiple times (in the correct order) and would still not solve #751 because this comparison would still fail.

@zloirock
Copy link
Owner

zloirock commented Apr 7, 2021

@Khartir too many other modules depend on es.regexp.exec, we should not enforce load es.regexp.constructor and es.regexp.sticky to modules that depend on it. So we should fix it without uniting those modules.

@zloirock
Copy link
Owner

zloirock commented Apr 7, 2021

I'll replace the check of regexpExec to RegExp.prototype.exec in maybeCallNative as a temporal fix - it will fix this issue, but RegExp -> String logic should be completely reworked.

@zloirock zloirock closed this in 49da8b7 Apr 7, 2021
@Khartir Khartir deleted the multiple-regexp-exec branch September 29, 2021 06:38
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.

IE11 + [email protected] String.prototype.split doesn't work with regexp
2 participants