create-react-app: "SVG as a component" doesn't quite work in 2.x

I’ll take a crack at an implementation after work hours, think this is important enough to exist before 2.0 launches

Hmm. I don’t think a macro that generates a webpack import is much better than just a webpack import. Both lock you into a non-standard system.

The strong side of the macro is that it can run without webpack. Coupling it to webpack removes that strong side.

The strong side of Babel+webpack (but no macro) is that it “looks” seamlessly.

I think combining macro+webpack doesn’t give us benefits of either approach.

I think let’s start with a userland solution based on babel-plugin-macros. Something like

import toReactComponent from 'svgr/macro';

const Logo = toReactComponent('./icon.svg');

<Logo />

If that works we can revert the new feature, and instead document this as a supported way.

OK, I have a different idea that can maybe solve both this and help https://github.com/facebookincubator/create-react-app/issues/3722 in the future.

My main concern with https://github.com/facebookincubator/create-react-app/issues/3722 is the build performance: we don’t want to spend time building something we don’t use. But the loader doesn’t know what needs to be built if it’s not directly in the “request” (of which the named import isn’t a part). And the same exact problem is what’s causing this bug.

I thought: what if we gave webpack more information without the user knowing about it? We could have an extra Babel transform that takes

import logoUrl, { ReactComponent as Logo } from './logo.svg';

and turns it into

// generated code
import logoUrl from './logo.svg';
import Logo from 'svgr-loader!./logo.svg';

(or similar)

The important part is this happens behind the scenes so the user doesn’t need to know about loaders. It does couple our Babel preset and webpack a little bit but it could be an opt-in option so people who use it without webpack won’t get this.

This way we can both have an expressive way to import what you need without webpack syntax, and a way for the loaders to know what exactly to generate. Inside CSS we’d always get the filenames (since we wouldn‘t transform it with Babel).

I won’t have time to work on this but @iansu maybe you’d be interested to try?

I feel like it’s okay for people who know what they’re doing, but we won’t be suggesting this as a default solution. It’s cool that @evenchange4 made something flexible for his use case though 😃

@evenchange4 we discourage using babel macros for svgs because they’ll inflate the bundle size (if you import the same svg across multiple files). Do you have a solution for this besides importing and exporting out of a separate file that’s used everywhere?

edit: this solution is still very cool & thank you for sharing

In my several projects, there are two kinds of SVG. One is icon-based which are exported from Adobe illustrator and the other is image-based (e.g., Logo) which are exported from Sketch.app.

I need to setup different configs of svgr for each SVG data source. The macro solution is supposed to be more configurable in this use case, and here is my full POC.

Logo componet: import one svg file with default config:

import toReactComponent from 'svgr.macro';
const Logo = toReactComponent('./logo.svg');

      ↓ ↓ ↓ ↓ ↓ ↓

const Logo = props => <svg width={116} height={28} viewBox="0 0 116 28" {...props}>
    <g fill="none" fillRule="evenodd">
      ...

Icon componets: glob multiple svg files with custom config:

const { DoneBlack, Autorenew } = toReactComponent(
  './material/*.svg',
  { icon: true, replaceAttrValues: ['#61DAFB=currentColor'] },
);

      ↓ ↓ ↓ ↓ ↓ ↓

const {
  DoneBlack,
  Autorenew
} = {
  "Autorenew": props => <svg height="1em" viewBox="0 0 24 24" width="1em" {...props}>
    ...
  </svg>,
  "DoneBlack": props => <svg height="1em" viewBox="0 0 24 24" width="1em" {...props}>
    ...
  </svg>
};

References

GitHub: https://github.com/evenchange4/svgr.macro Example: https://github.com/evenchange4/svgr.macro-example Demo: https://svgrmacro.netlify.com/

cc @iansu Do you still continue working on your branch into svgr repository?

This sounds good to me.

Two potential tweaks:

• We could consider not tying it to webpack, and instead providing a mapper for each type, e.g.

  {
    svg: {
      ReactComponent: path => `svgr-loader!${./path}`
    }
  }
  

  This would decouple the transform from webpack. Not sure it’s meaningful in any other context though. (Maybe we could use that in tests somehow?)

• I agree about not changing defaults. But another thing worth researching/considering is what happens when webpack supports ESM for asset loaders. (See @michael-ciniawsky comments above, https://github.com/facebookincubator/create-react-app/issues/3856#issuecomment-358686295). Would file-loader ever want to have a named export that might be useful? Our plugin would make it impossible to access that. But maybe that won’t happen. Worth following up with a discussion in file-loader repo; I’d appreciate if you filed an issue there.

@gaearon Not sure if this is solvable with the current CSS Pipeline and CJS (it’s overdue to update some parts there anyways (e.g css-loader)), but should be with ESM (see below). I’m working on next already it’s just not finished/ approved yet (~‘90%’) 😛

CSS Loader (css-loader@next (⚠️ not released))

https://github.com/webpack-contrib/css-loader/blob/next/src/index.js#L126

CSS => ESM (Formatting via css-loader)

main.css

@import './file.css'

.selector {
   background-image: url('./file.png');
}

module (main.css)

// CSS Imports
import CSS__URL__0 from './file.png' // url('./file.png') (Assets)
import CSS__IMPORT__0 from './file.css' // @import

// CSS Exports
// e.g CSS Modules (Named Exports)
export const name = 'file__selector---hash';

// CSS
export default `${css}`; // {String} (no css.toString() && 'runtime' anymore (needs triage))

CSS Plugin (extract-text-webpack-plugin@next (⚠️ not released))

https://gist.github.com/michael-ciniawsky/4c4427a185a1c33f065ea3069d144164

Extracts all CSS ‘Modules’ (ESM) per Chunk (Sync && Async)

chunk.css

/* file.css (@import) */
...css

/* main.css */
.selector {
   background-image: url('./3624tf43zziv.png');
}

file/url-loader

I write this up as gists, I have a few ideas via meta to enable something like ESM Exports/ isAsset but this needs use case analysis and triage. Could you open issues in css/url/file-loader to keep track of this ? If this isn’t to CRA specific (or we find generic ‘webpack’ pattens) it’s better to solve it in the loaders directly whenever possible 😃

Why not using Webpack issuer? @ppozniak had the same problem (https://github.com/smooth-code/svgr/issues/43) and it looks like issuer is a good workaround.

Saving it to a file (maybe in node_modules/.cache because that’ll be in the .gitignore?)

What if you run npm install or yarn and it deletes the cache? It feels flaky. I can’t easily describe in which scenarios it would break but I feel like it could.

Don’t solve it at all and wait to see if it’s a real problem? Maybe gzip will save us?

I doubt it, the minified variable names might be different. Also increasing parsed size is also bad.

@gaearon

I thought: what if we gave webpack more information without the user knowing about it? We could have an extra Babel transform that takes

import logoUrl, { ReactComponent as Logo } from './logo.svg';

and turns it into

// generated code
import logoUrl from './logo.svg';
import Logo from 'svgr-loader!./logo.svg';

I worked on an babel plugin that is doing that here. I wrote about my thoughts after implementing it in #3722

I’m experimenting with adding a macro to svgr. I’ve got the macro reading the SVG file and transforming it into a React component using svgr. I’m currently trying to figure out how to replace the macro function call with the generated code. I’m no babel expert so any help/suggestions would be welcome. Here’s the code: https://github.com/iansu/svgr/tree/babel-macro

(I can sense the excitement in @threepointone’s tone!)

You got it @gaearon 👍 I think that’s a good start to the solution for many problems like this.

So much for babel-plugin-macros being “experimental” 😉 😅