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

Switch from SVGOP Back to SVGO #104

Closed
Antonytm opened this issue Jan 27, 2022 · 4 comments
Closed

Switch from SVGOP Back to SVGO #104

Antonytm opened this issue Jan 27, 2022 · 4 comments

Comments

@Antonytm
Copy link
Contributor

Version of Dianoga

5.4.1

Environment description:

Sitecore 9

What configs you have enabled

Standard. Only Dianoga.Svg.config is enabled

Reproducible steps (1... 2... 3...) that cause the issue

  1. Try to pass --disable=removeViewBox argument to AdditionalToolArguments
			<dianogaOptimizeSvg>
				<!-- Optimize the SVG with SVGO https://github.com/svg/svgo -->
				<processor type="Dianoga.Optimizers.Pipelines.DianogaSvg.SvgoOptimizer, Dianoga">
					<ExePath>/App_Data/Dianoga Tools/SVGO/svgop.exe</ExePath>
					<AdditionalToolArguments>--disable=removeViewBox</AdditionalToolArguments>
				</processor>
			</dianogaOptimizeSvg>

This is configured based on examples in SvgOptimizerTests.cs.

What you expected to see, versus what you actually saw

Expected result:
viewBox property inside SVG file should not be touched.

Actual result:
viewBox property is removed.
If we look on https://github.com/twardoch/svgop/blob/master/src/app/svgop-pkg.js we understand why it doesn't work. Configs are baked in to .exe.

Relevant logs

No logs.

Test should fail

There is a test

		[Fact]
		public void ShouldSquishSmallSvgWithArgs()
		{
			Test(@"TestImages\small.svg",
				@"..\..\..\..\Dianoga\Dianoga Tools\SVGO\svgop.exe",
				"--disable=removeViewBox", out var args, out var startingSize);
			args.Stream.Length.Should().BeLessThan(startingSize).And.BeGreaterThan(0);
			args.IsOptimized.Should().BeTrue();
		}

that run svgop.exe with --disable=removeViewBox args. But if we check new StreamReader(args.Stream).ReadToEnd() we will see that viewBox was removed from test image.

My thoughts

  1. SVGOP is not supported. The latest release was on Apr 13, 2020. Latest release of SVGO was on Nov 02, 2021.
  2. SVGOP has no flexibility to pass parameters There is No Possibility to Pass Any Args to svgop.exe twardoch/svgop#7

I think that we need to revert back to SVGO from SVGOP.

@markgibbons25
Copy link
Collaborator

SVGO 2 has to be installed via NPM , it's not feasible to embed it into a nuget package anymore.
The only way I can think of doing it is to include installation instructions and tips for running it on a build server for package and deployment.

@Antonytm
Copy link
Contributor Author

Ok, I got you.

How about packaging svgo to executable. Nowadays, it is very easy to do using https://github.com/vercel/pkg ?

Nowadays it is very easy:
https://github.com/Antonytm/svgo-executable
You can review GitHub actions.

Basically, there are just 2 lines of code:

npm install //package.json contains 2 package: pkg and svgo
npx pkg node_modules/svgo //build svgo > exe

@Antonytm
Copy link
Contributor Author

I have created PR that is using svgo-win.exe that I got using the approach described above.
#105

markgibbons25 added a commit that referenced this issue Jan 30, 2022
@markgibbons25
Copy link
Collaborator

Great find thanks mate, released in 6.0.0-beta.2

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

2 participants