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

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

Closed
gnh1201 opened this issue Apr 24, 2021 · 19 comments
Closed

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

gnh1201 opened this issue Apr 24, 2021 · 19 comments

Comments

@gnh1201
Copy link

gnh1201 commented Apr 24, 2021

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
Copy link
Member

ljharb commented Apr 24, 2021

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: v4.5.14...v4.5.15

The only differences that could possibly cause an issue are 64b444e (es5-sham, avoiding an infinite loop in pre-proto browsers) and 0fda3b8 (adding a name to the split replacement), and 9390b33 (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
Copy link
Author

gnh1201 commented Apr 24, 2021

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
Copy link
Author

gnh1201 commented Apr 24, 2021

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

@gnh1201
Copy link
Author

gnh1201 commented Apr 24, 2021

#458

@ljharb
Copy link
Member

ljharb commented Apr 24, 2021

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

@ljharb
Copy link
Member

ljharb commented Apr 25, 2021

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

@gnh1201
Copy link
Author

gnh1201 commented Apr 25, 2021

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

@gnh1201
Copy link
Author

gnh1201 commented Apr 25, 2021

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

@gnh1201
Copy link
Author

gnh1201 commented Apr 25, 2021

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

@ljharb
Copy link
Member

ljharb commented Apr 25, 2021

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
Copy link
Author

gnh1201 commented Apr 26, 2021

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 64b444e.

  • es5-sham.js:L70
if (proto || proto == null) { // `undefined` is for pre-proto browsers    // <--- allow 'undefined' object
// if (proto || proto === null) //  <--- disallow 'undefined' object
  • es6-shim.js:L72
if (!force && name in object) { return; }    // <--- error and break (because `object' is `undefined`)

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

@ljharb
Copy link
Member

ljharb commented Apr 26, 2021

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

@gnh1201
Copy link
Author

gnh1201 commented Apr 26, 2021

I modified it as below.

  • es5-sham.js:L70
            if (proto || proto == null) { // `undefined` is for pre-proto browsers
                return proto || null;

But the results were the same.

@gnh1201
Copy link
Author

gnh1201 commented Apr 26, 2021

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

  • Before edit
* object: undefined   // <--- here
* name: string
_es6-shim iterator_
* value: function
function iterator() { return this; }
* force: undefined
  • After edit
* object: object   // <--- here
* name: string
_es6-shim iterator_
* value: function
function iterator() { return this; }
* force: undefined

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

@gnh1201
Copy link
Author

gnh1201 commented Apr 26, 2021

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

  • es5-sham.js:L70
            if (proto || typeof(proto) !== 'undefined') { // `undefined` is for pre-proto browsers
                return proto;

@ljharb
Copy link
Member

ljharb commented Apr 26, 2021

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

@gnh1201
Copy link
Author

gnh1201 commented Apr 27, 2021

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

@ljharb
Copy link
Member

ljharb commented Apr 27, 2021

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
Copy link
Author

gnh1201 commented Apr 28, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants