IE11 support

[dave-irvine]

Sorry to be the one that brings up IE support, but I've tracked down react-svg not working to here :(

https://github.com/tanem/svg-injector/blob/master/src/inject-element.ts#L85 uses Array.from which isn't in IE11 yet. I've got core-js in my webpack build but I'm using babel-preset-env with useBuiltins set to usage, but because your code comes from an npm module, webpack never checks it so I don't get the polyfill for Array.from.

Anything you can do to fix, or shall I just force the Array.from polyfill from core-js into my build?

[tanem]

Hey @dave-irvine, no worries, thanks for the report! Yep, I think in order for you to keep moving, it's best to include the polyfill for now.

I realise it's annoying you have to deal with this on your side though, so I'll have a think about ways we might be able to handle this within this lib (e.g. changing the build process or rewriting the source).

Will let you know how I get on.

[tanem]

@dave-irvine re:

I've got core-js in my webpack build but I'm using babel-preset-env with useBuiltins set to usage

Would it be possible to see your webpack config? Or better yet, a look at the app you're working on, if it's open source?

Asking because last time I used create react app (CRA), it defaulted to not ie <= 11 in it's browserslist config. Not sure if you're using CRA, but it'd be good to see what's going on in order to narrow the issue down.

[dave-irvine]

Hi @tanem, sorry for the delay on this. Please find repo here: https://github.com/dave-irvine/svg-injector-ie11

If you npm i and npm run dev you should get a server at localhost:8080 which you can open in Firefox (works!) and IE11 (doesn't work).

If you open the source and uncomment https://github.com/dave-irvine/svg-injector-ie11/blob/master/src/index.js#L14 so that babel-preset-env picks up me using Array.from and reload IE11, it will now work.

I guess you could just specify some targets in your babel-preset-env config? https://github.com/tanem/svg-injector/blob/master/rollup.config.js#L37

[tanem]

@dave-irvine no problem, thanks for taking the time to create that repo, it really made my life easier! Hopefully the following response gives you a path forward 🙏 (PS I realise you may be familiar with the things I talk about here, but I've kept the detail because I'll likely refer back to this issue in future when helping others).

To answer your question:

I guess you could just specify some targets in your babel-preset-env config? https://github.com/tanem/svg-injector/blob/master/rollup.config.js#L37

I did play around with this actually. In the end though, the reasons I don't want to do this are:

• I'd be making assumptions about what polyfills users would want to use, for example some might prefer es6-shim over core-js.
• I'd be adding extra code to the exported bundle for users who don't care about older browser support. It might be possible to eliminate any cruft via build tooling on the user side, but that would potentially open a big can of worms around something that's not a core concern of this library.
• It's fairly common to push this sort of thing into user-land, see here, here and here for example.

Recommendations

Here are three options ranging from least to most bloat:

1. Use just the Array.from() polyfill, e.g. import 'core-js/features/array/from';.
2. Use the IE11 polyfill from react-app-polyfill.
3. Use the full @babel/polyfill.

In terms of including the polyfill, the webpack docs have a nice approach via an external polyfills.js file here.

I'm guessing that this won't be the first such issue you'll hit if you're going to be running with IE11 in the context of a bigger React app. For that reason I'd recommend option 2 which seems like a nice middle ground.

What I'll do now

Update the documentation in both this library and react-svg so that it's clear you need to polyfill Array.from() if you're using older browsers.

[dave-irvine]

Beautiful package maintainer skills. A++ would raise issue again :)

[tanem]

😆 No problem, will update the docs shortly.

[tanem]

@dave-irvine just linked the two update doc PRs to this issue. Are you happy enough with those changes that I can close this issue now?

[dave-irvine]

👍 LGTM