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

IE11 support is broken #437

Closed
rubnig opened this issue Jul 15, 2019 · 34 comments
Closed

IE11 support is broken #437

rubnig opened this issue Jul 15, 2019 · 34 comments
Labels

Comments

@rubnig
Copy link

rubnig commented Jul 15, 2019

The README.md states that Shepherd still supports IE11, but the dist and demo (latest beta) give JavaScript errors. So Shepherd doesn't work with IE11 right now.

Is there a way to fix these issues or build Shepherd with IE11 support? There are still a lot of IE11 users on our platform (> 10%).

The lambda's are definitely breaking for IE11, see following example:

// returns true if 'el' should be allowed to receive touchmove events
  const allowTouchMove = el => locks.some(lock => {
    if (lock.options.allowTouchMove && lock.options.allowTouchMove(el)) {
      return true;
    }

    return false;
  });
@RobbieTheWagner
Copy link
Member

@rubnig what's the error in IE11?

@rubnig
Copy link
Author

rubnig commented Jul 15, 2019

@rwwagner90 Syntax error on the arrow function I gave as an example.

@RobbieTheWagner
Copy link
Member

@rubnig I need a specific example from the library, specifics on which file you are using, how you are importing etc.

@RobbieTheWagner
Copy link
Member

Oh, I see, I think this is from Tippy perhaps. Let me check our Babel config.

@RobbieTheWagner
Copy link
Member

@rubnig what was the last released version that works for you? Sorry, I do not have a good way to test in IE11 myself.

@rubnig
Copy link
Author

rubnig commented Jul 15, 2019

I was working with 2.10.0 before with the following polyfill for IE11:

if (!Object.entries) {
    Object.entries = function( obj ) {
        var ownProps = Object.keys( obj ),
            i = ownProps.length,
            resArray = new Array(i); // preallocate the Array
        while (i--)
            resArray[i] = [ownProps[i], obj[ownProps[i]]];

        return resArray;
    };
}

I think the JS dist is broken since version 3

@rubnig
Copy link
Author

rubnig commented Jul 15, 2019

@rwwagner90 Just tested the 3.0.0 version, since this version IE 11 doesn't work indeed.

@RobbieTheWagner
Copy link
Member

@rubnig sorry for breaking support! I have a PR open to also transpile our dependencies, so things will work in IE again. I also went ahead and added Object.assign and Object.entries polyfills from core-js, so hopefully that will get everything working again for you, and you will not need to add the Object.entries yourself either 😃

I want to ensure Shepherd is IE11 compatible out of the box, assuming you include no extra polyfills yourself.

I will release a new beta here shortly. Would you be able to help test it out please?

@rubnig
Copy link
Author

rubnig commented Jul 15, 2019

Yes off course, no problem @rwwagner90. Thanks for your fast response. I'll test this beta without any other code (just the pure Shepherd demo) so we can verify if all of the polyfills are working as expected.

I was already checking some things and I found out a problem with Symbol.iterator in function _iterableToArrayLimit(arr, i). Symbol is undefined in IE 11.

But I'll wait on your latest beta and do the testing for you.

@RobbieTheWagner
Copy link
Member

@rubnig can you give me a list of all the things that are undefined? I want to add polyfills for them, but I do not want to add all possible polyfills, just the ones we need.

@RobbieTheWagner
Copy link
Member

Actually, I think we can have core-js polyfill based on what things it sees we have used. Let me give that a try.

@RobbieTheWagner
Copy link
Member

@rubnig I tried the auto polyfill option, and it added a lot of weight to the package. I think we are better off manually adding the necessary polyfills. With that in mind, I am going to merge the PR, and get a new beta out, then we can determine what else is needed for IE11 support.

@rubnig
Copy link
Author

rubnig commented Jul 15, 2019

@rwwagner90 that's fine by me. I'll test the demo when it's ready and respond to this ticket with my findings.

@rubnig
Copy link
Author

rubnig commented Jul 15, 2019

@rwwagner90 it looks like that the require('core-js/stable/features/object/assign') and require('core-js/stable/features/object/entries') doesn't get included in the final build. Is this a problem with yarn?

@RobbieTheWagner
Copy link
Member

RobbieTheWagner commented Jul 15, 2019

@rubnig it seems like we might need to just bite the bullet and add all the polyfills that core-js thinks we need. It bloats the build quite a lot, but we need to get it to actually include the polyfills.

@RobbieTheWagner
Copy link
Member

@rubnig what are your thoughts on polyfills in general? Should that be up to the consuming app to add or should the library ship with them?

@RobbieTheWagner
Copy link
Member

@rubnig can you please try 4.0.0-beta.4?

@rubnig
Copy link
Author

rubnig commented Jul 16, 2019

@rwwagner90 just tried beta 4 and 5, still not working. I can clearly see some arrow functions in the output, which will definitely break IE 11. It looks like the Common JS plugin gives you ES6 output and Babel doesn't transform it to IE 11 ES5 compatible code.

what are your thoughts on polyfills in general? Should that be up to the consuming app to add or should the library ship with them?

That's a good question. I think if you state that a library works on certain browser, you should also provide the needed polyfills to let it work with this browser. Maybe it's a good idea to provide them in a seperate file like 'shepherd.polyfill.js' so a developer can decide to include them or use the project's own polyfills?

@RobbieTheWagner
Copy link
Member

@rubnig apologies, at some point yesterday, I had removed all the arrow functions, I added a new dep, auto-bind which seems to use them, and had to add that to the list for what is being transpiled.

I think, in most cases, people developing apps will be already including some sort of polyfills for IE11 support. Perhaps we should provide our own, and document that you should use it by itself, if you do not have a modern web app setup with babel and polyfills already, but if you are already adding core-js or other polyfills, reusing them and not including our file would probably be better.

@RobbieTheWagner
Copy link
Member

I think perhaps the easiest thing to do, might be to do a separate build and generate shepherd.ie11.js and shepherd.ie11.polyfill.js, that way the standard shepherd.min.js does not have the 11 kb min + gzip added to it that we need for IE11 support.

@RobbieTheWagner
Copy link
Member

@rubnig can you try 4.0.0-beta.6 please?

@rubnig
Copy link
Author

rubnig commented Jul 16, 2019

Totally agree with your approach @rwwagner90. Just had some time to verify the beta 6 and I can confirm it's working again on IE 11 now.

Just one small bug for showing the cross close sign:
image

This could be easily corrected by placing it with position absolute in the top right corner.

Thanks for your support!

@RobbieTheWagner
Copy link
Member

@rubnig that is odd with the "x" placement. Would you be interested in submitting a PR to fix it? The x is in the correct spot on Chrome, so I am not sure what changes to make.

@RobbieTheWagner
Copy link
Member

@rubnig I believe I may have fixed the x placement. Can you double check and make sure everything is good to go in IE11 still? If so, I will consider this closed.

@RobbieTheWagner
Copy link
Member

I got a VM to test IE in, and everything seems to work. I am going to close. Please let me know if you encounter any more issues!

@AmineMissaoui
Copy link

Hi @rwwagner90
I'm having the same issue with ie11, is the plugin still working on this browser?
Many thanks

@RobbieTheWagner
Copy link
Member

@AmineMissaoui no we do not support IE. You will have to ship all your own polyfills if you want to support it.

@AmineMissaoui
Copy link

@rwwagner90 how can i do that ? I'm not using a framework just js code.
Thanks

@RobbieTheWagner
Copy link
Member

@AmineMissaoui you will have to use babel, which you should be using anyway if you are supporting IE.

@AmineMissaoui
Copy link

@rwwagner90 I've tried to convert my js with babel but still not working, wich polyfills should i export? can you help please?

Thanks

@RobbieTheWagner
Copy link
Member

@AmineMissaoui again, we don't support IE11, you should use babel, then include babel polyfills and/or whatever polyfills you are missing for IE11. We do not have a list of the ones needed, but when you get an error it should tell you what polyfill you need.

@AmineMissaoui
Copy link

AmineMissaoui commented Nov 19, 2020

@rwwagner90 Thanks i tried it and it works, here's the polyfills that i have used if anyone needs it : https://polyfill.io/v3/polyfill.min.js?features=Promise%2CObject.assign%2CArray.from%2CArray.prototype.fill%2CObject.getOwnPropertyDescriptors

Cheers.

@yonkov
Copy link

yonkov commented Mar 24, 2021

Is there a version i can use that still supports Internet Explorer 11?

@RobbieTheWagner
Copy link
Member

@yonkov one of the 6.x or 7.x versions should work, but they also required polyfills. Our stance is to not support IE at all because Microsoft even has dropped support for it.

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

No branches or pull requests

4 participants