-
-
Notifications
You must be signed in to change notification settings - Fork 646
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
Comments
@rubnig what's the error in IE11? |
@rwwagner90 Syntax error on the arrow function I gave as an example. |
@rubnig I need a specific example from the library, specifics on which file you are using, how you are importing etc. |
Oh, I see, I think this is from Tippy perhaps. Let me check our Babel config. |
@rubnig what was the last released version that works for you? Sorry, I do not have a good way to test in IE11 myself. |
I was working with 2.10.0 before with the following polyfill for IE11:
I think the JS dist is broken since version 3 |
@rwwagner90 Just tested the 3.0.0 version, since this version IE 11 doesn't work indeed. |
@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 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? |
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 But I'll wait on your latest beta and do the testing for you. |
@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. |
Actually, I think we can have core-js polyfill based on what things it sees we have used. Let me give that a try. |
@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. |
@rwwagner90 that's fine by me. I'll test the demo when it's ready and respond to this ticket with my findings. |
@rwwagner90 it looks like that the |
@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. |
@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? |
@rubnig can you please try 4.0.0-beta.4? |
@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.
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? |
@rubnig apologies, at some point yesterday, I had removed all the arrow functions, I added a new dep, 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. |
I think perhaps the easiest thing to do, might be to do a separate build and generate |
@rubnig can you try 4.0.0-beta.6 please? |
@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. |
@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. |
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! |
Hi @rwwagner90 |
@AmineMissaoui no we do not support IE. You will have to ship all your own polyfills if you want to support it. |
@rwwagner90 how can i do that ? I'm not using a framework just js code. |
@AmineMissaoui you will have to use babel, which you should be using anyway if you are supporting IE. |
@rwwagner90 I've tried to convert my js with babel but still not working, wich polyfills should i export? can you help please? Thanks |
@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. |
@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. |
Is there a version i can use that still supports Internet Explorer 11? |
@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. |
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:
The text was updated successfully, but these errors were encountered: