paulmillr / es6-shim

ECMAScript 6 compatibility shims for legacy JS engines
http://paulmillr.com
MIT License
3.11k stars 387 forks source link

sticky flag polyfill #376

Closed Yaffle closed 8 years ago

Yaffle commented 8 years ago
(function (global) {
  var OriginalRegExp = global.RegExp;
  if (!("sticky" in OriginalRegExp.prototype)) {
    var originalExec = OriginalRegExp.prototype.exec;
    OriginalRegExp.prototype.exec = function (string) {
      var lastIndex = this.lastIndex;
      var result = originalExec.call(this, string);
      if (this._sticky && result != null && this.lastIndex - result[0].length !== lastIndex) {
        this.lastIndex = 0;
        return null;
      }
      return result;
    };
    global.RegExp = function (pattern, flags) {
      var i = flags != undefined ? flags.indexOf("y") : -1;
      var x = new OriginalRegExp(pattern, i === -1 ? flags : flags.slice(0, i) + flags.slice(i + 1) + (flags.indexOf("g") === -1 ? "g" : ""));
      x._sticky = i !== -1;
      return x;
    };
  }
}(this));
Yaffle commented 8 years ago

instead of RegExp#test you can use RegExp#exec(string) != null

This does not fix issues with String#match, String#search, String#split, String#replace... But 1) it should be strange to use those methods with sticky flag; 2) if you fix RegExp[@@match] and other methods to call RegExp#exec as per specification thouse, methods will be fixed automatically... 3) From my understanding of the specification both Firefox and Chrome works incorrectly for the next edge cases:

(seems, some methods in Firefox/Chrome work like with lastIndex == 0 and applied sticky flag)

console.log(
  "foo".match(function (e, i) { e.lastIndex = i; return e; }(/foo/y, 1)), // Chrome/Firefox: ["foo"], Edge: null
  "_foo".match(function (e, i) { e.lastIndex = i; return e; }(/foo/y, 1)), // Chrome/Firefox: null, Edge: ["foo"]
  "foo".replace(function (e, i) { e.lastIndex = i; return e; }(/foo/y, 1), "?"), // Chrome/Firefox: "?", Edge: "foo"
  "_foo".replace(function (e, i) { e.lastIndex = i; return e; }(/foo/y, 1), "?"), // Chrome/Firefox: "_foo", Edge: "_?"
  "foo".split(function (e, i) { e.lastIndex = i; return e; }(/foo/y, 1)), // Chrome/Firefox/Edge: ["", ""]
  "_foo".split(function (e, i) { e.lastIndex = i; return e; }(/foo/y, 1)) // Chrome/Firefox: ["_foo"], Edge: ["_",""]
);

Am I mistake?

ljharb commented 8 years ago

All of the Symbol methods do need to work with the sticky flags in an engine that implements them natively. I haven't finished implementing the full semantics of the symbol methods, as you point out - but I will soon.

Polyfilling sticky by a _sticky method isn't something that I'd find acceptable. However, those last test cases - in an environment that natively supports the y flag - do seem legitimate. Can you file those as a separate issue?

Yaffle commented 8 years ago

Polyfilling sticky by a _sticky method isn't something that I'd find acceptable.

@ljharb , I am replacing the constructor with custom one, which adds "_sticky" property - a very bad code, but it is simple and you are doing it too already, am i right? And I am replacing RegExp#exec ... which is not bad.

ljharb commented 8 years ago

I'm not talking about wrapping the constructor or replacing the methods - I'm not OK with adding publicly visible properties if they can be avoided. Since the sticky flag can't be polyfilled in native syntax, I don't think it'd be worth it.

hax commented 8 years ago

Any update to support sticky?

ljharb commented 8 years ago

@hax I'm still willing to consider it, but the effort to only partially support a regex flag doesn't seem worth it to me.