From b8281e7ad94bb357f90ca52308730b876d8a0991 Mon Sep 17 00:00:00 2001 From: Barak Igal Date: Mon, 4 Apr 2022 11:40:28 +0300 Subject: [PATCH 1/6] added ability to dedupe doplicated modules --- packages/webpack-plugin/src/index.ts | 1 - .../webpack-plugin/src/mini-css-support.ts | 6 ++- packages/webpack-plugin/src/plugin-utils.ts | 53 ++++++++++++++----- packages/webpack-plugin/src/plugin.ts | 24 +++++++-- packages/webpack-plugin/src/types.ts | 3 +- .../webpack-plugin/src/webpack-entities.ts | 2 +- .../e2e/duplicate-namespace-css-asset.spec.ts | 49 +++++++++++++++++ .../test/e2e/duplicate-namespace.spec.ts | 18 ++++++- .../projects/duplicate-namespace/src/index.js | 4 ++ .../duplicate-namespace/src/same-v1.st.css | 7 +++ .../duplicate-namespace/src/v1.st.css | 7 +++ .../webpack.config.css-output.js | 22 ++++++++ .../duplicate-namespace/webpack.config.js | 9 +++- 13 files changed, 180 insertions(+), 25 deletions(-) create mode 100644 packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts create mode 100644 packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/same-v1.st.css create mode 100644 packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/v1.st.css create mode 100644 packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js diff --git a/packages/webpack-plugin/src/index.ts b/packages/webpack-plugin/src/index.ts index 79bb9f7ec..f51c9d7a4 100644 --- a/packages/webpack-plugin/src/index.ts +++ b/packages/webpack-plugin/src/index.ts @@ -30,7 +30,6 @@ export { provideStylableModules, replaceCSSAssetPlaceholders, replaceMappedCSSAssetPlaceholders, - reportNamespaceCollision, staticCSSWith, uniqueFilterMap, } from './plugin-utils'; diff --git a/packages/webpack-plugin/src/mini-css-support.ts b/packages/webpack-plugin/src/mini-css-support.ts index ddc3cdfc6..8de9836cd 100644 --- a/packages/webpack-plugin/src/mini-css-support.ts +++ b/packages/webpack-plugin/src/mini-css-support.ts @@ -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', @@ -42,7 +46,7 @@ export function injectCssModules( dependencyTemplates, runtime: 'CSS' /*runtime*/, runtimeTemplate, - stylableBuildData: getStylableBuildData(stylableModules, module), + stylableBuildData, }) ), }); diff --git a/packages/webpack-plugin/src/plugin-utils.ts b/packages/webpack-plugin/src/plugin-utils.ts index d96697d87..98ff738ed 100644 --- a/packages/webpack-plugin/src/plugin-utils.ts +++ b/packages/webpack-plugin/src/plugin-utils.ts @@ -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({ @@ -365,27 +368,49 @@ export function getSortedModules(stylableModules: Map, namespaceToFileMapping: Map>, compilation: Compilation, - mode: 'ignore' | 'warnings' | 'errors' + mode: 'ignore' | 'warnings' | 'errors', + enableDedupe?: 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]; + + let common: { css: string; depth: number }; + const shouldReport = 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; + }); + 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 (enableDedupe) { + // 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; + } + } } } } diff --git a/packages/webpack-plugin/src/plugin.ts b/packages/webpack-plugin/src/plugin.ts index 263cb9eba..53173cb73 100644 --- a/packages/webpack-plugin/src/plugin.ts +++ b/packages/webpack-plugin/src/plugin.ts @@ -23,7 +23,7 @@ import { findIfStylableModuleUsed, staticCSSWith, getStylableBuildMeta, - reportNamespaceCollision, + handleNamespaceCollision, createOptimizationMapping, getTopLevelInputFilesystem, createDecacheRequire, @@ -51,7 +51,15 @@ 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 + */ + dedupe?: boolean; }; export interface StylableWebpackPluginOptions { @@ -136,6 +144,7 @@ const defaultOptimizations = (isProd: boolean): Required => ({ shortNamespaces: isProd, removeEmptyNodes: isProd, minify: isProd, + dedupe: false, }); const defaultOptions = ( @@ -321,6 +330,7 @@ export class StylableWebpackPlugin { const stylableBuildMeta: StylableBuildMeta = { depth: 0, isUsed: undefined, + isDuplicate: false, ...loaderData, }; module.buildMeta.stylable = stylableBuildMeta; @@ -418,6 +428,7 @@ export class StylableWebpackPlugin { css, isUsed: module.buildMeta.stylable.isUsed, depth: module.buildMeta.stylable.depth, + isDuplicate: module.buildMeta.stylable.isDuplicate, }); } }); @@ -434,12 +445,14 @@ export class StylableWebpackPlugin { const { usageMapping, namespaceMapping, namespaceToFileMapping } = createOptimizationMapping(sortedModules, optimizer); - reportNamespaceCollision( + handleNamespaceCollision( + stylableModules, namespaceToFileMapping, compilation, normalizeNamespaceCollisionOption( this.options.unsafeMuteDiagnostics.DUPLICATE_MODULE_NAMESPACE - ) + ), + optimizeOptions.dedupe ); for (const module of sortedModules) { @@ -509,7 +522,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) { diff --git a/packages/webpack-plugin/src/types.ts b/packages/webpack-plugin/src/types.ts index ec24a4075..2163dab2a 100644 --- a/packages/webpack-plugin/src/types.ts +++ b/packages/webpack-plugin/src/types.ts @@ -10,11 +10,12 @@ export interface StylableBuildMeta { isUsed: undefined | boolean; globals: Record; 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< diff --git a/packages/webpack-plugin/src/webpack-entities.ts b/packages/webpack-plugin/src/webpack-entities.ts index 0bf055a15..a48f530b3 100644 --- a/packages/webpack-plugin/src/webpack-entities.ts +++ b/packages/webpack-plugin/src/webpack-entities.ts @@ -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) { return; } if (this.cssInjection === 'js') { diff --git a/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts new file mode 100644 index 000000000..fac619ef8 --- /dev/null +++ b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts @@ -0,0 +1,49 @@ +import { StylableProjectRunner } from '@stylable/e2e-test-kit'; +import { expect } from 'chai'; +import { dirname } from 'path'; + +const project = 'duplicate-namespace'; +const projectDir = dirname( + require.resolve(`@stylable/webpack-plugin/test/e2e/projects/${project}/webpack.config`) +); + +describe(`(${project})`, () => { + const projectRunner = StylableProjectRunner.mochaSetup( + { + projectDir, + launchOptions: { + // headless: false, + }, + throwOnBuildError: false, + 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(); + + const { rulesLength, stylesLength } = await page.evaluate(() => { + const stylesLength = document.styleSheets.length; + const rulesLength = document.styleSheets[0].cssRules.length; + return { + stylesLength, + rulesLength, + }; + }); + + expect(stylesLength, 'stylable.css should exist').to.equal(1); + expect(rulesLength, 'sheet has 3 rules (one is omitted because duplication)').to.equal(3); + }); +}); diff --git a/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts b/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts index 51a4c21dd..2fd874f7f 100644 --- a/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts +++ b/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts @@ -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'; @@ -12,7 +12,7 @@ describe(`(${project})`, () => { { projectDir, launchOptions: { - // headless: false + // headless: false, }, throwOnBuildError: false, }, @@ -29,4 +29,18 @@ 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(); + + const styleElements = await page.evaluate(browserFunctions.getStyleElementsMetadata, { + includeCSSContent: false, + }); + + 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 + ]); + }); }); diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/index.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/index.js index 936daf1d4..17a73cca5 100644 --- a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/index.js +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/index.js @@ -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); diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/same-v1.st.css b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/same-v1.st.css new file mode 100644 index 000000000..7d4ca2695 --- /dev/null +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/same-v1.st.css @@ -0,0 +1,7 @@ +@namespace "V1"; + +.root { + font-size: 20px; +} + +/* st-namespace-reference="v1" */ diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/v1.st.css b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/v1.st.css new file mode 100644 index 000000000..7d4ca2695 --- /dev/null +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/src/v1.st.css @@ -0,0 +1,7 @@ +@namespace "V1"; + +.root { + font-size: 20px; +} + +/* st-namespace-reference="v1" */ diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js new file mode 100644 index 000000000..29877f9ba --- /dev/null +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js @@ -0,0 +1,22 @@ +const { StylableWebpackPlugin } = require('@stylable/webpack-plugin'); +const HtmlWebpackPlugin = require('html-webpack-plugin'); +const { join } = require('path'); + +/** @type {import('webpack').Configuration} */ +module.exports = { + mode: 'development', + context: __dirname, + devtool: 'source-map', + output: { + path: join(__dirname, 'dist2'), + }, + plugins: [ + new StylableWebpackPlugin({ + cssInjection: 'css', + optimize: { + dedupe: true, + }, + }), + new HtmlWebpackPlugin(), + ], +}; diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js index 4b6b84678..80421dfa0 100644 --- a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js @@ -6,5 +6,12 @@ module.exports = { mode: 'development', context: __dirname, devtool: 'source-map', - plugins: [new StylableWebpackPlugin(), new HtmlWebpackPlugin()], + plugins: [ + new StylableWebpackPlugin({ + optimize: { + dedupe: true, + }, + }), + new HtmlWebpackPlugin(), + ], }; From c9c7bcb32218963abdbcda2ae4f21d5b5665709b Mon Sep 17 00:00:00 2001 From: Barak Igal Date: Mon, 4 Apr 2022 13:52:55 +0300 Subject: [PATCH 2/6] refactor tests --- .../test/e2e/duplicate-namespace-css-asset.spec.ts | 9 ++++++--- .../webpack-plugin/test/e2e/duplicate-namespace.spec.ts | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts index fac619ef8..5968134cd 100644 --- a/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts +++ b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts @@ -32,7 +32,7 @@ describe(`(${project})`, () => { }); it('should only load one copy of duplicated module with same content and depth ', async () => { - const { page } = await projectRunner.openInBrowser(); + const { page } = await projectRunner.openInBrowser({ captureResponses: true }); const { rulesLength, stylesLength } = await page.evaluate(() => { const stylesLength = document.styleSheets.length; @@ -43,7 +43,10 @@ describe(`(${project})`, () => { }; }); - expect(stylesLength, 'stylable.css should exist').to.equal(1); - expect(rulesLength, 'sheet has 3 rules (one is omitted because duplication)').to.equal(3); + expect(stylesLength, 'stylable.css should exist').to.have.lengthOf(1); + expect( + rulesLength, + 'sheet has 3 rules (one is omitted because duplication)' + ).to.have.lengthOf(3); }); }); diff --git a/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts b/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts index 2fd874f7f..9d490ed56 100644 --- a/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts +++ b/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts @@ -31,7 +31,7 @@ describe(`(${project})`, () => { }); it('should only load one copy of duplicated module with same content and depth ', async () => { - const { page } = await projectRunner.openInBrowser(); + const { page } = await projectRunner.openInBrowser({ captureResponses: true }); const styleElements = await page.evaluate(browserFunctions.getStyleElementsMetadata, { includeCSSContent: false, @@ -39,7 +39,7 @@ describe(`(${project})`, () => { 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/same-v1.st.css', depth: '1' }, // duplicated only one survived { id: './src/index.st.css', depth: '2' }, // component import +1 depth ]); }); From 8bc195247e6fd848213f465867cbc7bc3dbb2d57 Mon Sep 17 00:00:00 2001 From: Barak Igal Date: Mon, 4 Apr 2022 14:12:04 +0300 Subject: [PATCH 3/6] change option key --- packages/webpack-plugin/src/plugin-utils.ts | 34 +++++++++++++-------- packages/webpack-plugin/src/plugin.ts | 7 +++-- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/webpack-plugin/src/plugin-utils.ts b/packages/webpack-plugin/src/plugin-utils.ts index 98ff738ed..a2d9c8869 100644 --- a/packages/webpack-plugin/src/plugin-utils.ts +++ b/packages/webpack-plugin/src/plugin-utils.ts @@ -373,7 +373,7 @@ export function handleNamespaceCollision( namespaceToFileMapping: Map>, compilation: Compilation, mode: 'ignore' | 'warnings' | 'errors', - enableDedupe?: boolean + dedupeSimilarStylesheets?: boolean ) { if (mode === 'ignore') { return; @@ -382,16 +382,10 @@ export function handleNamespaceCollision( if (resources.size > 1) { const modules = [...resources]; - let common: { css: string; depth: number }; - const shouldReport = 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; - }); + const shouldReport = dedupeSimilarStylesheets + ? areAllModulesTheSameCssAndDepth(stylableModules, modules) + : true; + if (shouldReport) { const resourcesReport = modules .map((module) => getModuleRequestPath(module, compilation)) @@ -404,7 +398,7 @@ export function handleNamespaceCollision( ); error.hideStack = true; compilation[mode].push(error); - } else if (enableDedupe) { + } 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; @@ -415,6 +409,22 @@ export function handleNamespaceCollision( } } +function areAllModulesTheSameCssAndDepth( + stylableModules: Map, + 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'; diff --git a/packages/webpack-plugin/src/plugin.ts b/packages/webpack-plugin/src/plugin.ts index 53173cb73..85ea4d9c8 100644 --- a/packages/webpack-plugin/src/plugin.ts +++ b/packages/webpack-plugin/src/plugin.ts @@ -58,8 +58,9 @@ type OptimizeOptions = OptimizeConfig & { /** * @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 */ - dedupe?: boolean; + dedupeSimilarStylesheets?: boolean; }; export interface StylableWebpackPluginOptions { @@ -144,7 +145,7 @@ const defaultOptimizations = (isProd: boolean): Required => ({ shortNamespaces: isProd, removeEmptyNodes: isProd, minify: isProd, - dedupe: false, + dedupeSimilarStylesheets: false, }); const defaultOptions = ( @@ -452,7 +453,7 @@ export class StylableWebpackPlugin { normalizeNamespaceCollisionOption( this.options.unsafeMuteDiagnostics.DUPLICATE_MODULE_NAMESPACE ), - optimizeOptions.dedupe + optimizeOptions.dedupeSimilarStylesheets ); for (const module of sortedModules) { From 6cebf5607cf6ac65e6dc1638ce209b93f9d2b8fc Mon Sep 17 00:00:00 2001 From: Barak Igal Date: Wed, 13 Apr 2022 12:18:11 +0300 Subject: [PATCH 4/6] fix test --- .../test/e2e/duplicate-namespace-css-asset.spec.ts | 4 ++-- .../projects/duplicate-namespace/webpack.config.css-output.js | 2 +- .../test/e2e/projects/duplicate-namespace/webpack.config.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts index 5968134cd..aa391f5c3 100644 --- a/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts +++ b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts @@ -43,10 +43,10 @@ describe(`(${project})`, () => { }; }); - expect(stylesLength, 'stylable.css should exist').to.have.lengthOf(1); + expect(stylesLength, 'only stylable.css should exist').to.equal(1); expect( rulesLength, 'sheet has 3 rules (one is omitted because duplication)' - ).to.have.lengthOf(3); + ).to.equal(3); }); }); diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js index 29877f9ba..c508c744a 100644 --- a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js @@ -14,7 +14,7 @@ module.exports = { new StylableWebpackPlugin({ cssInjection: 'css', optimize: { - dedupe: true, + dedupeSimilarStylesheets: true }, }), new HtmlWebpackPlugin(), diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js index 80421dfa0..d60202b07 100644 --- a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js @@ -9,7 +9,7 @@ module.exports = { plugins: [ new StylableWebpackPlugin({ optimize: { - dedupe: true, + dedupeSimilarStylesheets: true }, }), new HtmlWebpackPlugin(), From 93733ef1f17eaf8d37ecb00a1d3c976937da9088 Mon Sep 17 00:00:00 2001 From: Barak Igal Date: Wed, 13 Apr 2022 16:57:42 +0300 Subject: [PATCH 5/6] fix tests --- packages/e2e-test-kit/src/project-runner.ts | 5 +++++ .../test/e2e/duplicate-namespace-css-asset.spec.ts | 12 ++++++------ .../test/e2e/duplicate-namespace.spec.ts | 4 +--- .../duplicate-namespace/webpack.config.css-output.js | 6 +----- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/e2e-test-kit/src/project-runner.ts b/packages/e2e-test-kit/src/project-runner.ts index 852841fee..9b35381d1 100644 --- a/packages/e2e-test-kit/src/project-runner.ts +++ b/packages/e2e-test-kit/src/project-runner.ts @@ -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, diff --git a/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts index aa391f5c3..06a323919 100644 --- a/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts +++ b/packages/webpack-plugin/test/e2e/duplicate-namespace-css-asset.spec.ts @@ -1,13 +1,13 @@ import { StylableProjectRunner } from '@stylable/e2e-test-kit'; import { expect } from 'chai'; -import { dirname } from 'path'; +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})`, () => { +describe(`(${project}) css asset`, () => { const projectRunner = StylableProjectRunner.mochaSetup( { projectDir, @@ -15,6 +15,9 @@ describe(`(${project})`, () => { // headless: false, }, throwOnBuildError: false, + webpackOptions: { + output: { path: join(projectDir, 'dist2') }, + }, configName: 'webpack.config.css-output', }, before, @@ -44,9 +47,6 @@ describe(`(${project})`, () => { }); expect(stylesLength, 'only stylable.css should exist').to.equal(1); - expect( - rulesLength, - 'sheet has 3 rules (one is omitted because duplication)' - ).to.equal(3); + expect(rulesLength, 'sheet has 3 rules (one is omitted because duplication)').to.equal(3); }); }); diff --git a/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts b/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts index 9d490ed56..e947ef69e 100644 --- a/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts +++ b/packages/webpack-plugin/test/e2e/duplicate-namespace.spec.ts @@ -33,9 +33,7 @@ describe(`(${project})`, () => { 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, { - includeCSSContent: false, - }); + const styleElements = await page.evaluate(browserFunctions.getStyleElementsMetadata); expect(styleElements).to.eql([ { id: './src/same-index.st.css', depth: '1' }, // same content, different depth diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js index c508c744a..bf65a1631 100644 --- a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js @@ -1,20 +1,16 @@ const { StylableWebpackPlugin } = require('@stylable/webpack-plugin'); const HtmlWebpackPlugin = require('html-webpack-plugin'); -const { join } = require('path'); /** @type {import('webpack').Configuration} */ module.exports = { mode: 'development', context: __dirname, devtool: 'source-map', - output: { - path: join(__dirname, 'dist2'), - }, plugins: [ new StylableWebpackPlugin({ cssInjection: 'css', optimize: { - dedupeSimilarStylesheets: true + dedupeSimilarStylesheets: true, }, }), new HtmlWebpackPlugin(), From 561e9cd520befac3e51ac68612e7687512078777 Mon Sep 17 00:00:00 2001 From: Barak Igal Date: Tue, 27 Dec 2022 17:22:01 +0200 Subject: [PATCH 6/6] refactor: rename to experimentalDedupeSimilarStylesheets --- packages/webpack-plugin/src/plugin-utils.ts | 6 +++--- packages/webpack-plugin/src/plugin.ts | 8 ++++---- .../duplicate-namespace/webpack.config.css-output.js | 2 +- .../e2e/projects/duplicate-namespace/webpack.config.js | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/webpack-plugin/src/plugin-utils.ts b/packages/webpack-plugin/src/plugin-utils.ts index a2d9c8869..8566623a5 100644 --- a/packages/webpack-plugin/src/plugin-utils.ts +++ b/packages/webpack-plugin/src/plugin-utils.ts @@ -373,7 +373,7 @@ export function handleNamespaceCollision( namespaceToFileMapping: Map>, compilation: Compilation, mode: 'ignore' | 'warnings' | 'errors', - dedupeSimilarStylesheets?: boolean + experimentalDedupeSimilarStylesheets?: boolean ) { if (mode === 'ignore') { return; @@ -382,7 +382,7 @@ export function handleNamespaceCollision( if (resources.size > 1) { const modules = [...resources]; - const shouldReport = dedupeSimilarStylesheets + const shouldReport = experimentalDedupeSimilarStylesheets ? areAllModulesTheSameCssAndDepth(stylableModules, modules) : true; @@ -398,7 +398,7 @@ export function handleNamespaceCollision( ); error.hideStack = true; compilation[mode].push(error); - } else if (dedupeSimilarStylesheets) { + } else if (experimentalDedupeSimilarStylesheets) { // we keep the first one start index from 1 for (let i = 1; i < modules.length; i++) { getStylableBuildMeta(modules[i]).isDuplicate = true; diff --git a/packages/webpack-plugin/src/plugin.ts b/packages/webpack-plugin/src/plugin.ts index 85ea4d9c8..aff52a26c 100644 --- a/packages/webpack-plugin/src/plugin.ts +++ b/packages/webpack-plugin/src/plugin.ts @@ -58,9 +58,9 @@ type OptimizeOptions = OptimizeConfig & { /** * @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 + * This can be beneficial when you have multiple versions of the same package in your project */ - dedupeSimilarStylesheets?: boolean; + experimentalDedupeSimilarStylesheets?: boolean; }; export interface StylableWebpackPluginOptions { @@ -145,7 +145,7 @@ const defaultOptimizations = (isProd: boolean): Required => ({ shortNamespaces: isProd, removeEmptyNodes: isProd, minify: isProd, - dedupeSimilarStylesheets: false, + experimentalDedupeSimilarStylesheets: false, }); const defaultOptions = ( @@ -453,7 +453,7 @@ export class StylableWebpackPlugin { normalizeNamespaceCollisionOption( this.options.unsafeMuteDiagnostics.DUPLICATE_MODULE_NAMESPACE ), - optimizeOptions.dedupeSimilarStylesheets + optimizeOptions.experimentalDedupeSimilarStylesheets ); for (const module of sortedModules) { diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js index bf65a1631..6459602a2 100644 --- a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.css-output.js @@ -10,7 +10,7 @@ module.exports = { new StylableWebpackPlugin({ cssInjection: 'css', optimize: { - dedupeSimilarStylesheets: true, + experimentalDedupeSimilarStylesheets: true, }, }), new HtmlWebpackPlugin(), diff --git a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js index d60202b07..83369cc02 100644 --- a/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js +++ b/packages/webpack-plugin/test/e2e/projects/duplicate-namespace/webpack.config.js @@ -9,7 +9,7 @@ module.exports = { plugins: [ new StylableWebpackPlugin({ optimize: { - dedupeSimilarStylesheets: true + experimentalDedupeSimilarStylesheets: true, }, }), new HtmlWebpackPlugin(),