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

Browser version of 5.15.x does not work in IE 11 #11504

Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue bug ESLint is working incorrectly build This change relates to ESLint's build process

Comments

@spahnke
Copy link

spahnke commented Mar 13, 2019

Tell us about your environment

  • ESLint Version: 5.15.x
  • Node Version: 11.11.0
  • npm Version: 6.9.0

What did you do?

Built ESLint using

npm run webpack

and ran it in IE 11 (more precisely, in a web worker and in conjunction with the Monaco editor but that should not matter). The source code couldn't be parsed because it contains a lot of ES2015+ syntax like const and let since the recent change to Webpack. The Browserify version in 5.14.x works in IE 11.

You can get a step further by changing the Webpack config to also transpile files from node_modules (including .mjs files apparently) like shown below. This makes sure to transpile to ES5 but I got stuck at this stage because IE still throws the error Incorrect syntax in regular expression. Since I saw nothing obvious in the source code I gave up at this point.

rules: [
    {
        test: /\.m?js$/u,
        loader: "babel-loader",
        options: {
            presets: ["@babel/preset-env"]
        }
    }
]

So the question boils down to: What browsers do you intend to support with the browser variant of ESLint?

Are you willing to submit a pull request to fix this bug?
Not at the moment, sorry.

@spahnke spahnke added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Mar 13, 2019
@not-an-aardvark not-an-aardvark added build This change relates to ESLint's build process accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Mar 13, 2019
@not-an-aardvark
Copy link
Member

not-an-aardvark commented Mar 13, 2019

Thanks for the report, I can reproduce this issue. It seems like eslint-scope isn't getting compiled to ES5 with the new build setup.

@spahnke
Copy link
Author

spahnke commented Mar 16, 2019

Thanks so much for the fix!

It still doesn't quite work though. As I stated above, as soon as you load the ES5 compatible eslint.js in IE 11 you get the error Incorrect syntax in regular expression. It is reproducible with the following code. I don't know where this might be coming from as there is nothing super obvious in the code.

<script src="eslint.js"></script>
<script>
	var linter = new eslint();
	console.log(linter.verify("var foo = 1", {
		"parserOptions": {
			"ecmaVersion": 5
		},
		"rules": {
			"semi": "warn"
		}
	}));
</script>

image
image

P.S. Pardon the german error messages 😅

@g-plane
Copy link
Member

g-plane commented Mar 16, 2019

Can you paste the whole line? Thanks.

@spahnke
Copy link
Author

spahnke commented Mar 16, 2019

Sure, sorry about that!

Edit: Format the bad line to look like in Browserify

var $flags = __webpack_require__(199);

var $RegExp = global.RegExp;
var Base = $RegExp;
var proto = $RegExp.prototype;
var re1 = /a/g;
var re2 = /a/g; // "new" creates a new object, old webkit buggy here

var CORRECT_NEW = new $RegExp(re1) !== re1;

if (__webpack_require__(7) && (!CORRECT_NEW || __webpack_require__(8)(function () {
  re2[__webpack_require__(28)('match')] = false; // RegExp constructor can alter flags and IsRegExp works correct with @@match

  return $RegExp(re1) != re1 || $RegExp(re2) == re2 || $RegExp(re1, 'i') != '/a/i';
}))) {
  $RegExp = function RegExp(p, f) {
    var tiRE = this instanceof $RegExp;
    var piRE = isRegExp(p);
    var fiU = f === undefined;
    return !tiRE && piRE && p.constructor === $RegExp && fiU ? p 
      : inheritIfRequired(CORRECT_NEW 
      ? new Base(piRE && !fiU ? p.source : p, f) 
       : Base((piRE = p instanceof $RegExp) ? p.source : p, piRE && fiU ? $flags.call(p) : f)
       , tiRE ? this : proto, $RegExp);
  };

@g-plane
Copy link
Member

g-plane commented Mar 16, 2019

Did you build ESLint by yourself?

@spahnke
Copy link
Author

spahnke commented Mar 16, 2019

Yes, I pulled the current master branch and build ESLint by running

npm run webpack

@g-plane
Copy link
Member

g-plane commented Mar 16, 2019

Well, this is a bug from core-js.

Look at Line 4795 in the picture you gave. There is a comment which is RegExp constructor can alter flags and IsRegExp works correct with..., and I found it: https://github.com/zloirock/core-js/search?q=RegExp+constructor+can+alter+flags+and+IsRegExp+works+correct+with&unscoped_q=RegExp+constructor+can+alter+flags+and+IsRegExp+works+correct+with .

It seems that we're using core-js v2, but core-js v3 is developing now, so I'm not sure if they fixed it.

@spahnke
Copy link
Author

spahnke commented Mar 16, 2019

Hm, according to the changelog this was introduced 4 years ago

0.4.2 - 2015.01.10

  • Object static methods accept primitives
  • RegExp constructor can alter flags (IE9+)
  • Added Array.prototype[Symbol.unscopables]

Why did it work with Browserify in version 5.14.1 though?

Edit
Built with Browserify the same piece of code looks like this

var $flags = _dereq_(44);
var $RegExp = global.RegExp;
var Base = $RegExp;
var proto = $RegExp.prototype;
var re1 = /a/g;
var re2 = /a/g;
// "new" creates a new object, old webkit buggy here
var CORRECT_NEW = new $RegExp(re1) !== re1;

if (_dereq_(36) && (!CORRECT_NEW || _dereq_(42)(function () {
  re2[_dereq_(128)('match')] = false;
  // RegExp constructor can alter flags and IsRegExp works correct with @@match
  return $RegExp(re1) != re1 || $RegExp(re2) == re2 || $RegExp(re1, 'i') != '/a/i';
}))) {
  $RegExp = function RegExp(p, f) {
    var tiRE = this instanceof $RegExp;
    var piRE = isRegExp(p);
    var fiU = f === undefined;
    return !tiRE && piRE && p.constructor === $RegExp && fiU ? p
      : inheritIfRequired(CORRECT_NEW
        ? new Base(piRE && !fiU ? p.source : p, f)
        : Base((piRE = p instanceof $RegExp) ? p.source : p, piRE && fiU ? $flags.call(p) : f)
      , tiRE ? this : proto, $RegExp);
  };

Apart from the dependecy mangement, it does look the same to me...

@zloirock
Copy link

It's not a bug from core-js, it's a bug it regexp pattern, wrapper just calls internally original RegExp constructor with the passed pattern.

@aladdin-add
Copy link
Member

recently we are using /u flag in regex -- it may be related to this. can you see the error stack?

@aladdin-add aladdin-add reopened this Mar 16, 2019
@spahnke
Copy link
Author

spahnke commented Mar 17, 2019

@aladdin-add You are right!

It is the createGlobalLinebreakMatcher function in ast-utils.js. The u-flag was added in #11422.

image

This whole "syntax error" thing threw me off; for some reason I assumed this whole time that the error occurs during the parsing phase of the file 😩🤦‍♂️

@not-an-aardvark
Copy link
Member

I was under the impression that babel can polyfill the behavior of unicode regexes. Is that not the case?

@spahnke
Copy link
Author

spahnke commented Mar 17, 2019

Hm, it can in principle (Babel Regex). But I think there would be limitations for usages like

return new RegExp(LINEBREAK_MATCHER.source, "gu");
// or
new RegExp(somePatternVariable, someFlagVariable)

because it can't really manipulate the content of those variables, right?

@not-an-aardvark
Copy link
Member

I thought it would polyfill the RegExp constructor itself to handle those cases at runtime.

@zloirock
Copy link

It's too costly for runtime.

@spahnke
Copy link
Author

spahnke commented Mar 17, 2019

Babel apparently uses the regexpu package to do the transform which has this exact limitation. I couldn't find any polyfill of the RegExp contructor for the u-flag unfortunately.

@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Dec 10, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue bug ESLint is working incorrectly build This change relates to ESLint's build process
Projects
None yet
5 participants