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

fix(qwik-city): prefix ids of SVGs based on their path when loaded as qwik nodes #5497

Merged
merged 1 commit into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions packages/qwik-city/buildtime/vite/image-jsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import fs from 'node:fs';
import path from 'node:path';
import { parseId } from 'packages/qwik/src/optimizer/src/plugins/plugin';
import type { QwikCityVitePluginOptions } from './types';
import type { Config as SVGOConfig } from 'svgo';

/** @public */
export function imagePlugin(userOpts?: QwikCityVitePluginOptions): PluginOption[] {
Expand Down Expand Up @@ -111,8 +112,31 @@ export function optimizeSvg(
userOpts?: QwikCityVitePluginOptions
) {
const svgAttributes: Record<string, string> = {};
// note: would be great if it warned users if they tried to use qwik-default plugins, so that name collisions are avoided
const userPlugins = userOpts?.imageOptimization?.svgo?.plugins || [];
const prefixIdsConfiguration = userOpts?.imageOptimization?.svgo?.prefixIds;
const maybePrefixIdsPlugin: SVGOConfig['plugins'] =
prefixIdsConfiguration !== false ? [{ name: 'prefixIds', params: prefixIdsConfiguration }] : [];

const userPlugins =
userOpts?.imageOptimization?.svgo?.plugins?.filter((plugin) => {
if (
plugin === 'preset-default' ||
(typeof plugin === 'object' && plugin.name === 'preset-default')
) {
console.warn(
`You are trying to use the preset-default SVGO plugin. This plugin is already included by default, you can customize it through the defaultPresetOverrides option.`
);
return false;
}

if (plugin === 'prefixIds' || (typeof plugin === 'object' && plugin.name === 'prefixIds')) {
console.warn(
`You are trying to use the preset-default SVGO plugin. This plugin is already included by default, you can customize it through the prefixIds option.`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contains a typo though :/ The checked plugin name is "prefixIds", not "preset-default"

);
return false;
}

return true;
}) || [];

const data = optimize(code, {
floatPrecision: userOpts?.imageOptimization?.svgo?.floatPrecision,
Expand Down Expand Up @@ -144,6 +168,7 @@ export function optimizeSvg(
};
},
},
...maybePrefixIdsPlugin,
...userPlugins,
],
}).data;
Expand Down
25 changes: 10 additions & 15 deletions packages/qwik-city/buildtime/vite/image-jsx.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,22 @@ test('optimize svgs by path', () => {
optimizeSvg({ code: file.content, path: file.path })
);

assert.isTrue(
defaultOptimizedSvgs.every((svg) => svg.data.startsWith('<g><defs><linearGradient id="a"'))
// ids should have different names, because prefixIds plugin is enabled by default
assert.isFalse(
defaultOptimizedSvgs.some((svg) => svg.data.startsWith('<g><defs><linearGradient id="a"'))
);
});

const optimizedSvgsWithUserConfig = svgsFilesWithDefsTag.map((file) =>
test('prefixIds plugin should be disableable', () => {
const defaultOptimizedSvgs = svgsFilesWithDefsTag.map((file) =>
optimizeSvg(
{ code: file.content, path: file.path },
{
imageOptimization: {
svgo: {
plugins: ['prefixIds'],
},
},
}
{ imageOptimization: { svgo: { prefixIds: false } } }
)
);

// ids should have different names if prefixIds plugin is explicitly added by users
assert.isFalse(
optimizedSvgsWithUserConfig.some((svg) =>
svg.data.startsWith('<g><defs><linearGradient id="a"')
)
// all ids be optimized to "a" because prefixIds plugin is disabled
assert.isTrue(
defaultOptimizedSvgs.every((svg) => svg.data.startsWith('<g><defs><linearGradient id="a"'))
);
});
1 change: 1 addition & 0 deletions packages/qwik-city/buildtime/vite/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface ImageOptimizationOptions {
};
svgo?: Pick<SVGOConfig, 'floatPrecision' | 'multipass' | 'plugins'> & {
defaultPresetOverrides?: SVGOBuiltinPluginsWithOptionalParams['preset-default']['overrides'];
prefixIds?: SVGOBuiltinPluginsWithOptionalParams['prefixIds'] | false;
};
enabled?: boolean | 'only-production';
}
Expand Down