"SVG as a component" doesn't quite work in 2.x

[gaearon]

We merged #3718 and released the first 2.x alpha, but quickly discovered a few issues:

• Importing SVG from CSS is now broken, you get [Object object] in URLs.
• While that can be fixed by adding a named export called toString() here (which would just return the URL), it looks like the resulting JS bundle contains the React component as soon as CSS imports it. It's my impression that webpack CSS pipeline still relies on CommonJS and therefore can't tree shake the component.

I'm not sure if there's any solution even possible here 😢 We probably need to revert the feature.

Just in case, pinging @neoziro @iansu for more ideas.

[gaearon]

OK, I have a different idea that can maybe solve both this and help #3722 in the future.

My main concern with #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?

[gaearon]

@michael-ciniawsky

Any chance you could help us understand the best path forward here? It seems like we could avoid a hacky solution if file-loader and css-loader worked with ESM. Then svgr-loader/webpack could provide two exports (one for URL and one for a component), and when SVG is imported through CSS, it would only get the URL. Is this something that should work in the future? Is it blocked on webpack 4? Are there any hanging PRs that we could help with to push this feature through?

[michael-ciniawsky]

@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 :)

[michael-ciniawsky]

'Tree-shaking'/Dedupe works (so far), since the CSS is 💯 ESM then

[michael-ciniawsky]

It's also possible to add custom imports/exports to the CSS Module via postcss-loader && a postcss plugin

plugin.js

export default postcss.plugin('name', (options = {}) => (css, result) => {
  // Get Imports && Exports somehow...

  // CSS Import
  result.messages.push({
    type: 'import' // required (Message Type)
    name: `CSS__IMPORT__${idx}` // Can be anything
    url: 'path/to/file' // Can be anything
    import () { // required (Message Stringifier)
       return `import ${this.name} from '${this.url}';`
    }
  })
  // CSS Export
  result.messages.push({
    type: 'export', // required (Message Type)
    name: `${name}`, // Can be anything
    value: 'Custom Export', // Can be anything
    export () { // required (Message Stringifier)
       return `export const ${this.name} = ${this.value};`
    }
  })
})

[iansu]

It sounds like the Babel transform (either via a plugin or a macro) might be the best way forward. It also seems to be the favored solution in #3722 for implementing this for other file types.

[gaearon]

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.

[gaearon]

@kentcdodds @threepointone Maybe you can give a rough idea of how to implement this macro?

[gaearon]

I guess it'll be similar to this: #1792 (comment)