es-shims / es5-shim

ECMAScript 5 compatibility shims for legacy (and modern) JavaScript engines
MIT License
7.12k stars 899 forks source link

Conflict ES5-shim(sham) 4.5.15 and ES6-shim(sham) 0.35.x #476

Closed gnh1201 closed 3 years ago

gnh1201 commented 3 years ago

The recently released version of es5-shim 4.5.15 confirmed a conflict with es6-shim 0.35.x. It seems to be a new problem that did not occur in 4.5.14.

es5-shim(+sham) es6-shim(+sham) Is it works?
4.5.14 0.35.5 it works
4.5.14 0.35.6 it works
4.5.15 0.35.5 does not work
4.5.15 0.35.6 does not work

I'm going to take a closer look at what the difference is soon.

ljharb commented 3 years ago

I'm afraid I'll need a lot more info than "does not work" :-) I use both es5-shim and es6-shim on a number of projects, so I'm also surprised to hear there's an issue.

Here's the difference between v4.5.15 (which was released 3 months ago) and v4.5.14: https://github.com/es-shims/es5-shim/compare/v4.5.14...v4.5.15

The only differences that could possibly cause an issue are https://github.com/es-shims/es5-shim/commit/64b444ebddd81ccf04c86a8aed21fe754acc829b (es5-sham, avoiding an infinite loop in pre-proto browsers) and https://github.com/es-shims/es5-shim/commit/0fda3b895813dba67fedb6c2506c6c898760278f (adding a name to the split replacement), and https://github.com/es-shims/es5-shim/commit/9390b337000ba4263b33333beec453ce07c1f82a (caching floor/abs/pow off of Math).

The only thing I can think of is that perhaps es6-shim is patching these Math methods, which affects some of the other things in es5-shim.

gnh1201 commented 3 years ago

The lines that are currently experiencing errors in es6-shim are as follows. https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L72

  var defineProperty = function (object, name, value, force) {
    if (!force && name in object) { return; }
    if (supportsDescriptors) {
      Object.defineProperty(object, name, {
        configurable: true,
        enumerable: false,
        writable: true,
        value: value
      });
    } else {
      object[name] = value;
    }
  };

I checked the parameters passed. If es5-shim 4.5.15 is used, there is an entry in which the variable 'force' appears as 'undefined'.

* object: object
* name: string
indexOf
* value: function
function indexOf(searchElement) {
      var value = _arrayIndexOfApply(this, arguments);
      if (value === 0 && (1 / value) < 0) {
        return 0;
      }
      return value;
    }
* force: boolean
-1
* object: object
* name: string
_es6-shim iterator_
* value: function
function () { return this.values(); }
* force: undefined

* object: undefined
* name: string
_es6-shim iterator_
* value: function
function iterator() { return this; }
* force: undefined

// error

Based on the results above, there seems to be a problem with _iterator. When I used es5-shim 4.5.14, there were no items marked undefined and everything worked fine.

* object: function                                           
* name: string                                               
toString                                                     
* value: function                                            
function (){ return binder.apply(this, arguments); }         
* force: boolean                                             
-1                                                           
* object: object                                             
* name: string                                               
sup                                                          
* value: function                                            
function sub() { return ES.CreateHTML(this, 'sup', '', ''); }
* force: boolean                                             
-1                                                           
* object: function                                           
* name: string                                               
toString                                                     
* value: function                                            
function (){ return binder.apply(this, arguments); }         
* force: boolean                                             
-1                                                           

// done without error
gnh1201 commented 3 years ago

Oh! Fixed! I fixed this change back to === and it works fine. 64b444ebddd81ccf04c86a8aed21fe754acc829b

gnh1201 commented 3 years ago

https://github.com/es-shims/es5-shim/pull/458

ljharb commented 3 years ago

Thanks for clarifying; I’ll try to find a way to test this and avoid the infinite loop.

ljharb commented 3 years ago

@gnh1201 in which engines are you experiencing an error? is it when es6-shim is loaded, or when you invoke a specific method?

gnh1201 commented 3 years ago

The JavaScript engine uses Windows Scripting Host 5.812. There is a problem loading the library.

gnh1201 commented 3 years ago

Here is the file for the test. https://gist.github.com/gnh1201/3702353d163e549e097c2083b407f8fd

gnh1201 commented 3 years ago

This is an example of a project being used: https://github.com/gnh1201/welsonjs

ljharb commented 3 years ago

ah wow, WSH, that's a name i haven't heard in a long time :-)

I don't have access to any windows machines, so I'm not sure how i can test this. What's the exact failure you get, and on what line (please include the entire stack trace if present), of that gist?

gnh1201 commented 3 years ago

The exact point of failure is that there is a variable called object marked undefined. This depends on whether it is == or === as shown in 64b444ebddd81ccf04c86a8aed21fe754acc829b.

If I can see the stack trace, I will write it soon.

ljharb commented 3 years ago

What happens if you leave it as == null, but change the next line to return proto || null;?

gnh1201 commented 3 years ago

I modified it as below.

But the results were the same.

gnh1201 commented 3 years ago

I'm sorry. There is a difference when I check it again.

However, the variable force is marked undefined, so the type of error is the same.

gnh1201 commented 3 years ago

If I change it as below, the error will be solved.

ljharb commented 3 years ago

That change would disallow undefined (reverting 64b444ebddd81ccf04c86a8aed21fe754acc829b) but would also allow false, 0, NaN, and 0n.

gnh1201 commented 3 years ago

So what about proto || typeof(proto) === 'object' instead of proto || typeof(proto) !== 'undefined'?

ljharb commented 3 years ago

The previous behavior for that branch was “truthy or null”, and the infinite loop bug fix was to change it to “truthy or null or undefined”. Every suggestion you’ve made reverts the bugfix by disallowing undefined there.

gnh1201 commented 3 years ago

Modifying es6-shim to match the changes in es5-shim(sham) corrected the error. Please check if it is a valid solution.