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.

Sign up for GitHub

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

Jump to bottom

Various fixes and implementations to NativeRegExp #12

Closed
wants to merge 4 commits into from
Closed

Various fixes and implementations to NativeRegExp #12

wants to merge 4 commits into from

Conversation

duonglaiquang
Copy link

@duonglaiquang duonglaiquang commented Jan 4, 2024 •
edited
Loading

This PR does the following:

  1. Handle the case where RegExp.prototype.toString is called on NativeObject instead of NativeRegExp object (currently throwing TypeError: Method "toString" called on incompatible object)
  • These tests showcase the expected behavior of this method 
    <!DOCTYPE html>
<html>
<head>
<script>
console.log(RegExp.prototype.toString.call({})) // "/undefined/undefined"
console.log(RegExp.prototype.toString.call({source: "Foo"})) // "/Foo/undefined"
console.log(RegExp.prototype.toString.call({flags: "gy"})) // "/undefined/gy"
console.log(RegExp.prototype.toString.call({source: "Foo", flags: "g"})) // "/Foo/g"
console.log(RegExp.prototype.toString.call({source: "Foo", flags: "g", sticky: true})) // "/Foo/g"
</script>
</head>
<body>
</body>
</html>

  1. Implement RegExp.dotAll flag

  2. Implement String.prototype.replaceAll

  3. Fix an issue when String.includes/startsWith/endsWith throw TypeError when the first argument is a regex, even if Symbol.match of that regex has been set to false.

    This function is also used to identify if objects have the behavior of regular expressions. For example, the methods String.prototype.startsWith(), String.prototype.endsWith() and String.prototype.includes(), check if their first argument is a regular expression and will throw a TypeError if they are. Now, if the match symbol is set to false (or a Falsy value except undefined), it indicates that the object is not intended to be used as a regular expression object.

  • These tests showcase the buggy behavior of the current implementation. 
    <!DOCTYPE html>
<html>
<head>
<script>
var regExp = /./;
try {
  console.log("/./".includes(regExp))
} catch (e) {
  console.log(e); // TypeError: First argument to String.prototype.includes must not be a regular expression
  regExp[Symbol.match] = false;
  console.log("/./".includes(regExp)) // expected: true, got TypeError
}
</script>
</head>
<body>
</body>
</html>

@duonglaiquang duonglaiquang changed the title Fix unwanted TypeError when calling RegExp.prototype.toString Various fixes and implementations to NativeRegExp Jan 4, 2024
@duonglaiquang duonglaiquang mentioned this pull request Jan 4, 2024
Closed
@rbri
Copy link
Member

rbri commented Jan 4, 2024

And again many thanks, will make a Rhino PR out of this and update core-js directly.
Will inform if a new snapshot build is available

This was referenced Jan 5, 2024
Closed
Merged
@rbri
Copy link
Member

rbri commented Jan 5, 2024

@duonglaiquang,
have done

Hope your are fine with all this, many thanks
Ronald

And all the best for 2024!

@rbri rbri closed this Jan 5, 2024
@duonglaiquang duonglaiquang deleted the duong_regexp branch March 14, 2024 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@duonglaiquang @rbri