Disable viewBox removal in SVGO

[ampersarnie]

Description

Fixes #32 - SVG's can have viewBox stripped out of them which will cause the appearance of cropping when the SVGs are displayed. This was caused by SVGO which is used by SVGR for optimisation. By default SVGO will strip out any instances of viewBox causing the cropping effect. To prevent this from happening we can disable the feature in the SVGR config.

Change Log

• Adds SVGR config to disable viewBox removal in SVGO.

[g-elwell]

@ampersarnie thanks for looking into this! There were a couple of minor changes required to get it working.

The proper syntax for the svgoConfig was a bit hard to track down!

The order of the loaders matters because url-loader transforms files into Base64 string, so we're currently importing SVGs like so:

import { ReactComponent as MailIcon } from '../static/images/mail-icon.svg';

Without url-loader being last, the above code will produce an error and the correct way to import an SVG would become:

import MailIcon from '../static/images/mail-icon.svg';

We're doing a bit of a dance by converting SVG to a string and back into a React component, but I assume there's a reason for it. Using the Base64 string as an img src maybe? SVGR take a different approach to this which might be worth bearing in mind for future and enables the simpler import syntax.

Anyway this works perfectly.

Tested in dev and prod modes

• SVG imports continue to work as expected
• ViewBox attribute remains present on an SVG if width and height values match it

[ampersarnie]

@g-elwell That's actually really interesting to know. I wasn't expecting the position change to have that kind of impact. How would you prefer that to work, because I can move it around if it's going to be too much of an inconvenience? My option from what you've described is to leave this as it is as this reads a lot better;

import MailIcon from '../static/images/mail-icon.svg';

As for the backward and forwards conversion, that actually wasn't something that had come up. We needed the option to implement SVGs on a component level and this is how it was implemented without any insight on the side effect. It's possibly something that needs investigating further to avoid that conversion. So thanks for the resources on that.

[g-elwell]

The reason the order matters (and apologies as I skipped over this) is because if url-loader is placed earlier, it will convert the SVG into a string and SVGR will no longer be able to process it.

How would you prefer that to work, because I can move it around if it's going to be too much of an inconvenience? My option from what you've described is to leave this as it is as this reads a lot better;

import MailIcon from '../static/images/mail-icon.svg';

I also prefer this syntax, but in order to enable it we'd have to introduce a breaking change since existing SVG imports using the other syntax will stop working and there are a few projects that I know of which are already using them. That said it would be a very simple fix to get them working again.

As it stands this PR will fix the viewBox issue without any side-effects so I'd recommend merging as-is and I'm happy to look into things further on #34 including whether there are any other improvements to be made around how we're handling SVGs

[ampersarnie]

Thanks for the clarification @g-elwell!

in order to enable it we'd have to introduce a breaking change since existing SVG imports using the other syntax will stop working and there are a few projects that I know of which are already using them. That said it would be a very simple fix to get them working again.

As we're not out of RC we should expect a degree of breaking change, so not overly concerned about that at the moment. Just a risk we have.

I'm happy to look into things further on #34 including whether there are any other improvements to be made around how we're handling SVGs

That would be much appreciated!