Skip to content
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;
Comment thread
barak007 marked this conversation as resolved.
Outdated
};

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
Copy Markdown

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(),
],
};