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

Corrupted icons after update from 3.1 to 4.0 #219

Closed
EugeneDraitsev opened this issue Nov 5, 2018 · 7 comments
Closed

Corrupted icons after update from 3.1 to 4.0 #219

EugeneDraitsev opened this issue Nov 5, 2018 · 7 comments

Comments

@EugeneDraitsev
Copy link

EugeneDraitsev commented Nov 5, 2018

🐛 Bug Report

After update @svgr/webpack from version 3.1 to 4.0 some of icons looks corrupted. I'm using unpacked CRA 2.0 with default config.

To Reproduce

Use this svgs on version 3.1 and then on version 4.0:

<svg viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
<path d="M25,5h-3V3c0-1.7-1.3-3-3-3H5C3.3,0,2,1.3,2,3v20c0,1.7,1.3,3,3,3h4v1c0,2.2,1.8,4,4,4h12c2.2,0,4-1.8,4-4V9
	C29,6.8,27.2,5,25,5z M5,24c-0.6,0-1-0.5-1-1V3c0-0.6,0.4-1,1-1h14c0.5,0,1,0.4,1,1v2h-6.3H13H6.3c-0.6,0-1,0.4-1,1s0.4,1,1,1h3.2
	C9.4,7.3,9.2,7.7,9.1,8C9.1,8,9,8,9,8H6.5c-0.6,0-1,0.4-1,1s0.4,1,1,1H9v3c-0.1,0-0.1,0-0.2,0H6.1c-0.6,0-1,0.4-1,1s0.4,1,1,1h2.7
	c0.1,0,0.1,0,0.2,0V16c-0.1,0-0.1,0-0.2,0H6.1c-0.6,0-1,0.4-1,1c0,0.6,0.4,1,1,1h2.7c0.1,0,0.1,0,0.2,0V19c-0.1,0-0.1,0-0.2,0H6.1
	c-0.6,0-1,0.4-1,1s0.4,1,1,1h2.7c0.1,0,0.1,0,0.2,0v3H5z M27,27c0,1.1-0.9,2-2,2H13c-1.1,0-2-0.9-2-2V9c0-1.1,0.9-2,2-2h0.7H25
	c1.1,0,2,0.9,2,2V27z M25.2,19c0,0.6-0.4,1-1,1H13.4c-0.6,0-1-0.4-1-1s0.4-1,1-1h10.7C24.7,18,25.2,18.4,25.2,19z M25.2,22
	c0,0.6-0.4,1-1,1H13.4c-0.6,0-1-0.4-1-1s0.4-1,1-1h10.7C24.7,21,25.2,21.4,25.2,22z M25.2,25c0,0.6-0.4,1-1,1H13.4c-0.6,0-1-0.4-1-1
	s0.4-1,1-1h10.7C24.7,24,25.2,24.4,25.2,25z M12.3,11c0-0.6,0.4-1,1-1h7.3c0.6,0,1,0.4,1,1s-0.4,1-1,1h-7.3
	C12.8,12,12.3,11.6,12.3,11z M16,13c0.6,0,1,0.4,1,1s-0.4,1-1,1h-2.5c-0.6,0-1-0.4-1-1s0.4-1,1-1H16z"/>
</svg>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32">
    <path d="M10,29c-0.3,0-0.6-0.1-0.8-0.3l-6-7c-0.3-0.3-0.3-0.7-0.1-1.1S3.6,20,4,20h2V5c0-0.6,0.4-1,1-1h6
	c0.6,0,1,0.4,1,1v15h2c0.4,0,0.7,0.2,0.9,0.6s0.1,0.8-0.1,1.1l-6,7C10.6,28.9,10.3,29,10,29z M6.2,22l3.8,4.5l3.8-4.5H13
	c-0.6,0-1-0.4-1-1V6H8v15c0,0.6-0.4,1-1,1H6.2z M25,29h-6c-0.6,0-1-0.4-1-1V13h-2c-0.4,0-0.7-0.2-0.9-0.6c-0.2-0.4-0.1-0.8,0.1-1.1
	l6-7c0.4-0.4,1.1-0.4,1.5,0l6,7c0.3,0.3,0.3,0.7,0.1,1.1C28.7,12.8,28.4,13,28,13h-2v15C26,28.6,25.6,29,25,29z M20,27h4V12
	c0-0.6,0.4-1,1-1h0.8L22,6.5L18.2,11H19c0.6,0,1,0.4,1,1V27z"
    />
</svg>
<svg viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
    <path d="M24,30C24,30,24,30,24,30L8,29.9c-1.7,0-3-1.4-3-3V16l4-3.2V5c0-1.7,1.3-3,3-3h12c1.6,0,3,1.3,3,3v22
	c0,0.8-0.3,1.6-0.9,2.1C25.6,29.7,24.8,30,24,30z M7,17v9.9c0,0.6,0.5,1,1,1L24,28c0,0,0,0,0,0c0.3,0,0.5-0.1,0.7-0.3
	c0.2-0.2,0.3-0.4,0.3-0.7V5c0-0.5-0.4-1-1-1H12c-0.5,0-1,0.5-1,1v8.7L7,17z M15,6h-2v7h2V6z M19,6h-2v7h2V6z M23,6h-2v7h2V6z"
    />
</svg>

Steps to reproduce the behavior:

Expected behaviour

3.1:
image

Actual behaviour

4.0:
image

Link to repl or repo (highly encouraged)

Please provide a minimal repository on GitHub.
https://github.com/EugeneDraitsev/svgo-4.0-corrupted-icons
you can change @svgr/webpack from 4.0 to 3.1 to see results

## System:
 - OS: macOS 10.14
 - CPU: x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
 - Memory: 86.02 MB / 16.00 GB
 - Shell: 5.3 - /bin/zsh
## Binaries:
 - Node: 10.7.0 - ~/.nvm/versions/node/v10.7.0/bin/node
 - Yarn: 1.10.1 - /usr/local/bin/yarn
 - npm: 6.3.0 - ~/.nvm/versions/node/v10.7.0/bin/npm
 - Watchman: 4.9.0 - /usr/local/bin/watchman
@EugeneDraitsev
Copy link
Author

Seems like I found the reason of issue. My icons contains \n and \t in d attribute of path.
My webpack config:

{
  test: /\.(js|mjs|jsx|ts|tsx)$/,
  include: paths.appSrc,
  loader: require.resolve('babel-loader'),
  options: {
    customize: require.resolve(
      'babel-preset-react-app/webpack-overrides',
    ),

    plugins: [
      [
        require.resolve('babel-plugin-named-asset-import'),
        {
          loaderMap: {
            svg: {
              ReactComponent: '@svgr/webpack?-prettier![path]',
            },
          },
        },
      ],
    ],
    // This is a feature of `babel-loader` for webpack (not Babel itself).
    // It enables caching results in ./node_modules/.cache/babel-loader/
    // directory for faster rebuilds.
    cacheDirectory: true,
    // Don't waste time on Gzipping the cache
    cacheCompression: false,
  },
},

@svgr/webpack version 3.1 with this config ignores\n and \t symbols, but in version 4.0 this symbols exist in path:

image

If I remove svgo option in webpack, it works correct

  svg: {
    ReactComponent: '@svgr/webpack?-prettier![path]',
  }

I'm not sure what behavior is correct, but At least it works different on 3.1 and 4.0 versions.

@gregberge
Copy link
Owner

gregberge commented Nov 8, 2018

Hello,@EugeneDraitsev,

First I tried to reproduced the issue with your repository but some scripts are missing:

Error: Cannot find module '/Users/neoziro/Desktop/svgo-4.0-corrupted-icons/scripts/start.js'

I tried to run svgr v4 on your provided SVG and it works fine. It looks like a SVGO issue but it is very weird that the problem does not happen in [email protected] because the SVGO version hasn't changed between the two versions...

@EugeneDraitsev
Copy link
Author

EugeneDraitsev commented Nov 8, 2018

Hi, @neoziro

I added missing files to my repo and now it works fine (I just forgot to add files after yarn eject of create-react-app). You can just pull changes and try it again.

@gregberge
Copy link
Owner

@EugeneDraitsev it should be fixed in v4.0.1.

@EugeneDraitsev
Copy link
Author

EugeneDraitsev commented Nov 8, 2018

@neoziro, thanks for that fast fix. \n symbols is removed now, but \t symbols are stills exist in an svg. I think your regex should be like:

const LINE_BREAKS_REGEXP = /[\t\r\n\u0085\u2028\u2029]+/g;

Also I updated my repo from example and test regex above and seems like it works with it

@gregberge
Copy link
Owner

@EugeneDraitsev yes, since \t are not a problem, I decided to let them.

@EugeneDraitsev
Copy link
Author

@neoziro but it still fails with \t
4.0.1
image
4.0.1 with const LINE_BREAKS_REGEXP = /[\t\r\n\u0085\u2028\u2029]+/g;
image

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

No branches or pull requests

2 participants