Skip to content
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

Incompatibility with file-loader version 5 #367

Closed
barroudjo opened this issue Dec 11, 2019 · 4 comments
Closed

Incompatibility with file-loader version 5 #367

barroudjo opened this issue Dec 11, 2019 · 4 comments
Labels
v5 Released in v5

Comments

@barroudjo
Copy link

Hello,

Just to let you know, I have found a case where you have an incompatibility with file-loader version 5, than can easily be solved by a small modification on your side.
You enable loading SVGs both as react components and good old files with file-loader / url-loader. The thing is that in that case you expect the output of file-loader to be a commonJs type of export, hence the regular expression you have here: https://github.com/smooth-code/svgr/blob/1915aee289fcbe29237dca1b94e8309b971549f3/packages/webpack/src/index.js#L34

However the last version of file-loader (v5) now outputs by default an ESM type of export, which does not work with this regex anymore. So the webpack plugin thinks it's actually reading the SVG, and of course parsing that ESM export as an SVG fails.

The not so explicit error one gets when launching webpack is the following:

ERROR in ./src/images/menu_burger.svg
Module build failed (from ./node_modules/@svgr/webpack/lib/index.js):
NonErrorEmittedError: (Emitted value instead of an instance of Error) Error in parsing SVG: Non-whitespace before first tag.
Line: 0
Column: 1
Char: e
    at D:\Projects\ELS\sie-efl-blackpearl-laquotidienne\node_modules\webpack\lib\NormalModule.js:313:13
    at D:\Projects\ELS\sie-efl-blackpearl-laquotidienne\node_modules\loader-runner\lib\LoaderRunner.js:367:11
    at D:\Projects\ELS\sie-efl-blackpearl-laquotidienne\node_modules\loader-runner\lib\LoaderRunner.js:233:18
    at context.callback (D:\Projects\ELS\sie-efl-blackpearl-laquotidienne\node_modules\loader-runner\lib\LoaderRunner.js:111:13)
    at D:\Projects\ELS\sie-efl-blackpearl-laquotidienne\node_modules\@svgr\webpack\lib\index.js:74:58

The good news is it can be solved without a change on your side, by setting file-loader to use old commonJs exports (https://github.com/webpack-contrib/file-loader#esmodule). But of course it would be nice if that was not required !

Thanks for this great library !

@azangru
Copy link

azangru commented Dec 13, 2019

#362 is a related issue.

As far as I understood from @evilebottnawi 's explanation in this thread, webpack-contrib/file-loader#351, forcing file-loader to use commonjs modules instead of esmodules will increase the file size of the output.

@barroudjo
Copy link
Author

Exactly. Which is why I explain in the OP that this can be solved in a fairly simple manner by updating the regex there: https://github.com/smooth-code/svgr/blob/1915aee289fcbe29237dca1b94e8309b971549f3/packages/webpack/src/index.js#L34
I guess I could just do a PR instead of saying it's possible...

@zhangriyueming
Copy link

Same problem. The workaround fix is:

-  const previousExport = exportMatches ? `export default ${exportMatches[1]}` : null;
+  const previousExport = exportMatches ? `export default ${exportMatches[1]}` : 
+  source.toString('utf-8').startsWith('export') ? source : null;

gregberge added a commit that referenced this issue Dec 18, 2019
@gregberge gregberge added the v5 Released in v5 label Dec 23, 2019
@gregberge
Copy link
Owner

Will be fixed in v5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v5 Released in v5
Projects
None yet
Development

No branches or pull requests

4 participants