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

feat(webpack-plugin): ability to dedupe duplicated modules #2426

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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
5 changes: 5 additions & 0 deletions packages/e2e-test-kit/src/project-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,11 @@ export class ProjectRunner {
protected loadWebpackConfig(): webpack.Configuration {
const config = require(join(this.testDir, this.options.configName || 'webpack.config'));
const loadedConfig = config.default || config;
if (loadedConfig.output?.path) {
throw new Error(
'output.path is not allowed in webpack.config.js when running with project-runner. please use webpackOptions.output.path in the spec file instead.'
);
}
return {
...loadedConfig,
...this.options.webpackOptions,
Expand Down
1 change: 0 additions & 1 deletion packages/webpack-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export {
provideStylableModules,
replaceCSSAssetPlaceholders,
replaceMappedCSSAssetPlaceholders,
reportNamespaceCollision,
staticCSSWith,
uniqueFilterMap,
} from './plugin-utils';
Expand Down
6 changes: 5 additions & 1 deletion packages/webpack-plugin/src/mini-css-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ export function injectCssModules(
const chunkGraph = compilation.chunkGraph;

for (const [module] of stylableModules) {
const stylableBuildData = getStylableBuildData(stylableModules, module);
if (stylableBuildData.isDuplicate) {
continue;
}
const cssModule = new CssModule({
context: module.context,
identifier: module.resource.replace(/\.st\.css$/, '.css') + '?stylable-css-inject',
Expand All @@ -42,7 +46,7 @@ export function injectCssModules(
dependencyTemplates,
runtime: 'CSS' /*runtime*/,
runtimeTemplate,
stylableBuildData: getStylableBuildData(stylableModules, module),
stylableBuildData,
})
),
});
Expand Down
63 changes: 49 additions & 14 deletions packages/webpack-plugin/src/plugin-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,10 @@ export function createStaticCSS(
dependencyTemplates: DependencyTemplates
) {
const cssChunks = Array.from(stylableModules.keys())
.filter((m) => getStylableBuildMeta(m).isUsed !== false)
.filter((m) => {
const buildMeta = getStylableBuildMeta(m);
return buildMeta.isUsed !== false && !buildMeta.isDuplicate;
})
.sort((m1, m2) => getStylableBuildMeta(m1).depth - getStylableBuildMeta(m2).depth)
.map((m) => {
return replaceMappedCSSAssetPlaceholders({
Expand Down Expand Up @@ -365,31 +368,63 @@ export function getSortedModules(stylableModules: Map<NormalModule, BuildData |
});
}

export function reportNamespaceCollision(
export function handleNamespaceCollision(
stylableModules: Map<NormalModule, BuildData | null>,
namespaceToFileMapping: Map<string, Set<NormalModule>>,
compilation: Compilation,
mode: 'ignore' | 'warnings' | 'errors'
mode: 'ignore' | 'warnings' | 'errors',
dedupeSimilarStylesheets?: boolean
) {
if (mode === 'ignore') {
return;
}
for (const [namespace, resources] of namespaceToFileMapping) {
if (resources.size > 1) {
const resourcesReport = [...resources]
.map((module) => getModuleRequestPath(module, compilation))
.join('\n');

const error = new compilation.compiler.webpack.WebpackError(
`Duplicate namespace ${JSON.stringify(
namespace
)} found in multiple different resources:\n${resourcesReport}\nThis issue indicates multiple versions of the same library in the compilation, or different paths importing the same stylesheet like: "esm" or "cjs".`
);
error.hideStack = true;
compilation[mode].push(error);
const modules = [...resources];

const shouldReport = dedupeSimilarStylesheets
? areAllModulesTheSameCssAndDepth(stylableModules, modules)
: true;

if (shouldReport) {
const resourcesReport = modules
.map((module) => getModuleRequestPath(module, compilation))
.join('\n');

const error = new compilation.compiler.webpack.WebpackError(
`Duplicate namespace ${JSON.stringify(
namespace
)} found in multiple different resources:\n${resourcesReport}\nThis issue indicates multiple versions of the same library in the compilation, or different paths importing the same stylesheet like: "esm" or "cjs".`
);
error.hideStack = true;
compilation[mode].push(error);
} else if (dedupeSimilarStylesheets) {
// we keep the first one start index from 1
for (let i = 1; i < modules.length; i++) {
getStylableBuildMeta(modules[i]).isDuplicate = true;
getStylableBuildData(stylableModules, modules[i]).isDuplicate = true;
}
}
}
}
}

function areAllModulesTheSameCssAndDepth(
stylableModules: Map<NormalModule, BuildData | null>,
modules: NormalModule[]
) {
let common: { css: string; depth: number };
return modules.some((mod) => {
const { css, depth } = getStylableBuildData(stylableModules, mod);
if (!common) {
common = { css, depth };
} else if (common.css !== css || common.depth !== depth) {
return true;
}
return false;
});
}

export function normalizeNamespaceCollisionOption(opt?: boolean | 'warn') {
if (opt === true) {
return 'ignore';
Expand Down
25 changes: 21 additions & 4 deletions packages/webpack-plugin/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
findIfStylableModuleUsed,
staticCSSWith,
getStylableBuildMeta,
reportNamespaceCollision,
handleNamespaceCollision,
createOptimizationMapping,
getTopLevelInputFilesystem,
createDecacheRequire,
Expand Down Expand Up @@ -51,7 +51,16 @@ import { parse } from 'postcss';
import { getWebpackEntities, StylableWebpackEntities } from './webpack-entities';

type OptimizeOptions = OptimizeConfig & {
/**
* Enable css minification
*/
minify?: boolean;
/**
* @experimental
* Remove modules with the same target css and same depth from output
* This can be beneficial when you have multiple versions of the same package in your project
*/
dedupeSimilarStylesheets?: boolean;
barak007 marked this conversation as resolved.
Show resolved Hide resolved
};

export interface StylableWebpackPluginOptions {
Expand Down Expand Up @@ -136,6 +145,7 @@ const defaultOptimizations = (isProd: boolean): Required<OptimizeOptions> => ({
shortNamespaces: isProd,
removeEmptyNodes: isProd,
minify: isProd,
dedupeSimilarStylesheets: false,
});

const defaultOptions = (
Expand Down Expand Up @@ -321,6 +331,7 @@ export class StylableWebpackPlugin {
const stylableBuildMeta: StylableBuildMeta = {
depth: 0,
isUsed: undefined,
isDuplicate: false,
...loaderData,
};
module.buildMeta.stylable = stylableBuildMeta;
Expand Down Expand Up @@ -418,6 +429,7 @@ export class StylableWebpackPlugin {
css,
isUsed: module.buildMeta.stylable.isUsed,
depth: module.buildMeta.stylable.depth,
isDuplicate: module.buildMeta.stylable.isDuplicate,
});
}
});
Expand All @@ -434,12 +446,14 @@ export class StylableWebpackPlugin {
const { usageMapping, namespaceMapping, namespaceToFileMapping } =
createOptimizationMapping(sortedModules, optimizer);

reportNamespaceCollision(
handleNamespaceCollision(
stylableModules,
namespaceToFileMapping,
compilation,
normalizeNamespaceCollisionOption(
this.options.unsafeMuteDiagnostics.DUPLICATE_MODULE_NAMESPACE
)
),
optimizeOptions.dedupeSimilarStylesheets
);

for (const module of sortedModules) {
Expand Down Expand Up @@ -509,7 +523,10 @@ export class StylableWebpackPlugin {
getEntryPointModules(entryPoint, compilation.chunkGraph, (module) => {
const m = module as NormalModule;
if (stylableModules.has(m)) {
modules.set(m, getStylableBuildData(stylableModules, m));
const buildData = getStylableBuildData(stylableModules, m);
if (!buildData.isDuplicate) {
modules.set(m, buildData);
}
}
});
if (modules.size) {
Expand Down
3 changes: 2 additions & 1 deletion packages/webpack-plugin/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ export interface StylableBuildMeta {
isUsed: undefined | boolean;
globals: Record<string, boolean>;
unusedImports: string[];
isDuplicate: boolean;
}

export type BuildData = Pick<
StylableBuildMeta,
'css' | 'namespace' | 'depth' | 'exports' | 'isUsed' | 'urls'
'css' | 'namespace' | 'depth' | 'exports' | 'isUsed' | 'urls' | 'isDuplicate'
>;

export type LoaderData = Pick<
Expand Down
2 changes: 1 addition & 1 deletion packages/webpack-plugin/src/webpack-entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export function getWebpackEntities(webpack: Compiler['webpack']): StylableWebpac
}: DependencyTemplateContext
) {
const stylableBuildData = getStylableBuildData(this.stylableModules, module);
if (!stylableBuildData.isUsed) {
if (!stylableBuildData.isUsed || stylableBuildData.isDuplicate) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check the injection of our runtime functions

return;
}
if (this.cssInjection === 'js') {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { StylableProjectRunner } from '@stylable/e2e-test-kit';
import { expect } from 'chai';
import { dirname, join } from 'path';

const project = 'duplicate-namespace';
const projectDir = dirname(
require.resolve(`@stylable/webpack-plugin/test/e2e/projects/${project}/webpack.config`)
);

describe(`(${project}) css asset`, () => {
const projectRunner = StylableProjectRunner.mochaSetup(
{
projectDir,
launchOptions: {
// headless: false,
},
throwOnBuildError: false,
webpackOptions: {
output: { path: join(projectDir, 'dist2') },
},
configName: 'webpack.config.css-output',
},
before,
afterEach,
after
);

it('should report detailed path of duplicate namespaced files', () => {
const errors = projectRunner.getBuildErrorMessages();
expect(errors.length).to.equal(1);
const { message } = errors[0];
expect(message).to.includes('Duplicate namespace');
expect(message).to.includes('./src/index.js\n ./src/index.st.css <-- Duplicate');
expect(message).to.includes('./src/index.js\n ./src/same-index.st.css <-- Duplicate');
});

it('should only load one copy of duplicated module with same content and depth ', async () => {
const { page } = await projectRunner.openInBrowser({ captureResponses: true });

const { rulesLength, stylesLength } = await page.evaluate(() => {
const stylesLength = document.styleSheets.length;
const rulesLength = document.styleSheets[0].cssRules.length;
return {
stylesLength,
rulesLength,
};
});

expect(stylesLength, 'only stylable.css should exist').to.equal(1);
expect(rulesLength, 'sheet has 3 rules (one is omitted because duplication)').to.equal(3);
});
});
16 changes: 14 additions & 2 deletions packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { StylableProjectRunner } from '@stylable/e2e-test-kit';
import { browserFunctions, StylableProjectRunner } from '@stylable/e2e-test-kit';
import { expect } from 'chai';
import { dirname } from 'path';

Expand All @@ -12,7 +12,7 @@ describe(`(${project})`, () => {
{
projectDir,
launchOptions: {
// headless: false
// headless: false,
},
throwOnBuildError: false,
},
Expand All @@ -29,4 +29,16 @@ describe(`(${project})`, () => {
expect(message).to.includes('./src/index.js\n ./src/index.st.css <-- Duplicate');
expect(message).to.includes('./src/index.js\n ./src/same-index.st.css <-- Duplicate');
});

it('should only load one copy of duplicated module with same content and depth ', async () => {
const { page } = await projectRunner.openInBrowser({ captureResponses: true });

const styleElements = await page.evaluate(browserFunctions.getStyleElementsMetadata);

expect(styleElements).to.eql([
{ id: './src/same-index.st.css', depth: '1' }, // same content, different depth
{ id: './src/same-v1.st.css', depth: '1' }, // duplicated only one survived
{ id: './src/index.st.css', depth: '2' }, // component import +1 depth
]);
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { classes as a } from './index.st.css';
import { classes as b } from './same-index.st.css';
import { classes as v1 } from './v1.st.css';
import { classes as v1Same } from './same-v1.st.css';

document.documentElement.classList.add(a.root);
document.documentElement.classList.add(b.root);
document.documentElement.classList.add(v1.root);
document.documentElement.classList.add(v1Same.root);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@namespace "V1";

.root {
font-size: 20px;
}

/* st-namespace-reference="v1" */
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@namespace "V1";

.root {
font-size: 20px;
}

/* st-namespace-reference="v1" */
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const { StylableWebpackPlugin } = require('@stylable/webpack-plugin');
const HtmlWebpackPlugin = require('html-webpack-plugin');

/** @type {import('webpack').Configuration} */
module.exports = {
mode: 'development',
context: __dirname,
devtool: 'source-map',
plugins: [
new StylableWebpackPlugin({
cssInjection: 'css',
optimize: {
dedupeSimilarStylesheets: true,
},
}),
new HtmlWebpackPlugin(),
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,12 @@ module.exports = {
mode: 'development',
context: __dirname,
devtool: 'source-map',
plugins: [new StylableWebpackPlugin(), new HtmlWebpackPlugin()],
plugins: [
new StylableWebpackPlugin({
optimize: {
dedupeSimilarStylesheets: true
},
}),
new HtmlWebpackPlugin(),
],
};