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

Latest version of SVGO 1.1.0 breaks svg-sprite #276

Closed
longzheng opened this issue Sep 17, 2018 · 15 comments
Closed

Latest version of SVGO 1.1.0 breaks svg-sprite #276

longzheng opened this issue Sep 17, 2018 · 15 comments

Comments

@longzheng
Copy link

svgo just released 1.1.0 on npm which seems to break svg-sprite.

It seems to have broken a lot of other third-party tools as well, I believe this is related svg/svgo#1030

(node:9800) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'distribute' of undefined
    at SVGSpriterQueue.remove (C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\lib\svg-sprite\queue.js:97:58)
    at C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\node_modules\async\dist\async.js:473:16
    at next (C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\node_modules\async\dist\async.js:5329:29)
    at C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\node_modules\async\dist\async.js:969:16
    at C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\lib\svg-sprite.js:200:9
    at C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\node_modules\async\dist\async.js:473:16
    at next (C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\node_modules\async\dist\async.js:5329:29)
    at C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\node_modules\async\dist\async.js:969:16
    at C:\Users\longz\AppData\Roaming\npm\node_modules\svg-sprite\lib\svg-sprite\transform\svgo.js:92:3
    at <anonymous>
(node:9800) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:9800) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.```
@jonnitto
Copy link

It is fixed with this version: https://github.com/svg/svgo/releases/tag/v1.1.1 svg-sprite should raise the dependency to ^1.1.1

@jkphl
Copy link
Collaborator

jkphl commented Sep 17, 2018

@jonnitto Awesome, thank you! Will do as soon as I find some minutes.

@jonnitto
Copy link

Thank you for your work!

@AKrishma
Copy link

Please update if https://github.com/svg/svgo/releases/tag/v1.1.1 tag has been updated in package.json of svg-sprite of 1.4.0 tag. Since we are using gulp-svg-sprite 1.4.0 and which in turn using svg-sprite 1.4.0. This is causing lot of build breaks in multiple projects. So, early response from you will be highly appreciated.

@longzheng
Copy link
Author

@AKrishma I think you should be able to fix your local by clearing your node_modules so it downloads fresh packages (it should grab the latest svgo now which has the fix)

@AKrishma
Copy link

@longzheng, provided worked for us.
Thankyou all for quick workaround.

@jkphl
Copy link
Collaborator

jkphl commented Sep 18, 2018

I'm just working on updating the dependencies, especially the SVGO one. Unfortunately, updating SVGO doesn't fix the package tests for me, I'm still getting these UnhandledPromiseRejectionWarnings. I did clear the node_modules directory and re-installed all packages, but this didn't help. Do you happen to have done anything else that could help?

jkphl added a commit that referenced this issue Sep 18, 2018
@jkphl
Copy link
Collaborator

jkphl commented Sep 18, 2018

@longzheng Will try (although SVGO already seems to be successfully updated to 1.1.1). Also, seems that some other dependencies require higher node versions in the meantime, so this might even end up in a version 1.5 for svg-sprite ... I'm on it.

@longzheng
Copy link
Author

Sorry my bad, I misread your message.

@longzheng
Copy link
Author

Just putting it out there, maybe another of the dependencies you updated is causing a different issue now? Have you tried just doing one build where you only update svgo?

@jkphl
Copy link
Collaborator

jkphl commented Sep 18, 2018

@longzheng Yeah, tried that, but there's no difference. The error messages are still about the same UnhandledPromiseRejectionWarning (builds for all node versions are failing, see https://travis-ci.org/jkphl/svg-sprite, although the tests vor Node 4 and 5 are failing for different reasons). So SVGO probably isn't the true / only reason. I'll be having a meeting in 15 minutes, but I'll try to fix this as soon as possible. If you've got any further ideas I'd highly appreciate them in order to speed things up.

@longzheng
Copy link
Author

longzheng commented Sep 18, 2018

Ok, I'm a node noob but I figured out the actual cause of the exception was not svgo.

I checked out your commit b9a2df3702d6f7322d6f0bc3df2edcdab88a9a14

When running the tests, this line https://github.com/jkphl/svg-sprite/blob/b9a2df3702d6f7322d6f0bc3df2edcdab88a9a14/lib/svg-sprite/transform/svgo.js#L83

was causing an exception

TypeError: Cannot read property 'level' of undefined
    at C:\Users\longz\Desktop\test\jkphl\svg-sprite\lib\svg-sprite\transform\svgo.js:83:45
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)"

Inspecting spriter.config.log.transports it looks like this is not an object but an array.

screen shot 2018-09-18 at 6 16 07 pm 2

I have no context of what this is or why it has been changed (another dependency breaks it?) but that's the actual error.

If i change to spriter.config.log.transports[0].level it all works, but I'm not sure that's the intended behaviour.

@longzheng
Copy link
Author

Set up a demo #277

The tests still fail, but it's failing on sprites not matching.

@jkphl
Copy link
Collaborator

jkphl commented Sep 18, 2018

@longzheng Awesome, thanks for the pointer! This must be it! I'm just back from my meeting and will look at it asap. Looks like this is something related to winston (the logger used by svg-sprite). Btw, errors because of non-matching sprites are usually harmless and the result of minor (= sub-pixel) rendering changes in image comparing libraries.

@jkphl jkphl closed this as completed in b84d8fd Sep 18, 2018
@jkphl
Copy link
Collaborator

jkphl commented Sep 18, 2018

I just pushed out the release 1.4.1 with only minimal dependency updates which should fix the SVGO issue. Gulp & Grunt versions will follow shortly. It turned out that updating all dependencies caused a separate problem (which looked very similar though) — I'll do that along with a 1.5.0 release (as I'll have to drop support for Node.js < 6.4 then as well). Thanks for all your help!

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

No branches or pull requests

4 participants