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

Normative: Don't call well-known Symbol methods for RegExp on primitive values #3009

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

petamoriken
Copy link

Fixed that methods for RegExp, such as @@split, are not called on primitive values.

Background

The internal code of runtimes such as Node.js and Deno are implemented that they work without problems even if they are polluted with prototypes. With this normative change in place, those runtimes don't need to worry if changes to String.prototype[Symbol.split] are polluted.

On the other hand, if this normative change is not approved, the following workaround will have to be provided:

// primordial
const StringPrototypeSplit = Function.call.bind(String.prototype.split);

// safe wrapper
export const SafeStringPrototypeSplit = (str, separator, limit) => {
  if (typeof separator === "string") {
    return StringPrototypeSplit(str, {
      __proto__: null,
      toString() { return separator },
    }, limit);
  }
  return StringPrototypeSplit(str, separator, limit);
};

This approach could cause performance issue.

downstream: denoland/deno#17473

@ljharb ljharb added normative change Affects behavior required to correctly evaluate some ECMAScript source text needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Feb 5, 2023
@ljharb
Copy link
Member

ljharb commented Feb 5, 2023

@petamoriken can someone from deno add this to the March meeting agenda?

@petamoriken
Copy link
Author

@lucacasonato Would you work on this issue?

@petamoriken petamoriken force-pushed the normative/dont-call-regexp-@@method-on-primitives branch from ca31671 to 25424af Compare March 2, 2023 08:05
@bathos
Copy link
Contributor

bathos commented Mar 2, 2023

I can see that this change would cause this to no longer evaluate to “surprise”:

String.prototype[Symbol.split] = () => "surprise";
"a.b.c".split(".");

I see no obvious downside to the proposed change in a vacuum, but given it’s a breaking change, I’m curious whether there’s been research yet on whether it’s web compatible in practice?

(It seems plausible that it could be safe, but it also seems plausible that faithful polyfill implementations, e.g. @zloirock’s core-js, could be impacted).

@zloirock
Copy link

zloirock commented Mar 2, 2023

core-js does not detect delegation to @@methods for primitives, only for objects, so it's enough safe.

@petamoriken petamoriken force-pushed the normative/dont-call-regexp-@@method-on-primitives branch from f9d3b02 to c45ae45 Compare November 26, 2024 16:24
@petamoriken petamoriken changed the title Normative: Don't call @@method for RegExp on primitive values Normative: Don't call well-known Symbol methods for RegExp on primitive values Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants