Skip to content

Commit 31afe26

Browse files
Tim Lindvalltimlindvall
Tim Lindvall
authored andcommitted
feat: Class name collision detection.
- Throw an error if a generated class name collides with app CSS. - Add convenience config option to omit class names from the optimizer. - Add short class name to test fixture as a smoke test.
1 parent 3ec0216 commit 31afe26

File tree

6 files changed

+168
-20
lines changed

6 files changed

+168
-20
lines changed

packages/@css-blocks/ember-app/src/broccoli-plugin.ts

+101-17
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { OptiCSSOptions, Optimizer, parseSelector, postcss } from "opticss";
1212
import * as path from "path";
1313

1414
import { RuntimeDataGenerator } from "./RuntimeDataGenerator";
15-
import { cssBlocksPostprocessFilename, cssBlocksPreprocessFilename } from "./utils/filepaths";
15+
import { cssBlocksPostprocessFilename, cssBlocksPreprocessFilename, optimizedStylesPostprocessFilepath, optimizedStylesPreprocessFilepath } from "./utils/filepaths";
1616
import { AddonEnvironment } from "./utils/interfaces";
1717

1818
const debug = debugGenerator("css-blocks:ember-app");
@@ -47,7 +47,7 @@ export class CSSBlocksApplicationPlugin extends Filter {
4747
let factory = new BlockFactory(config, postcss);
4848
let analyzer = new EmberAnalyzer(factory, this.cssBlocksOptions.analysisOpts);
4949
let optimizerOptions = this.cssBlocksOptions.optimization;
50-
this.reserveClassnames(optimizerOptions);
50+
this.reserveClassnames(optimizerOptions, this.cssBlocksOptions.appClasses);
5151
let optimizer = new Optimizer(optimizerOptions, analyzer.optimizationOptions);
5252
let blocksUsed = new Set<Block>();
5353
for (let entry of entries) {
@@ -98,6 +98,22 @@ export class CSSBlocksApplicationPlugin extends Filter {
9898
this.output.writeFileSync(optLogFileName, optimizationResult.actions.logStrings().join("\n"), "utf8");
9999
debug("Wrote css, sourcemap, and optimization log.");
100100

101+
// Also, write out a list of generated classes that we can use later
102+
// for conflict detection during postprocess.
103+
const classesUsed: Set<string> = new Set();
104+
optimizationResult.styleMapping.optimizedAttributes.forEach(attr => {
105+
classesUsed.add(attr.value.valueOf());
106+
});
107+
this.output.writeFileSync(
108+
optimizedStylesPreprocessFilepath,
109+
JSON.stringify(
110+
new Array(...classesUsed.values()),
111+
undefined,
112+
" ",
113+
),
114+
);
115+
debug("Wrote list of generated classes.");
116+
101117
let dataGenerator = new RuntimeDataGenerator([...blocksUsed], optimizationResult.styleMapping, analyzer, config, reservedClassnames);
102118
let data = dataGenerator.generate();
103119
let serializedData = JSON.stringify(data, undefined, " ");
@@ -116,7 +132,7 @@ export class CSSBlocksApplicationPlugin extends Filter {
116132
* application to the the list of identifiers that should be omitted by the
117133
* classname generator.
118134
*/
119-
reserveClassnames(optimizerOptions: Partial<OptiCSSOptions>): void {
135+
reserveClassnames(optimizerOptions: Partial<OptiCSSOptions>, appClassesAlias: string[]): void {
120136
let rewriteIdents = optimizerOptions.rewriteIdents;
121137
let rewriteIdentsFlag: boolean;
122138
let omitIdents: Array<string>;
@@ -131,7 +147,8 @@ export class CSSBlocksApplicationPlugin extends Filter {
131147
omitIdents = rewriteIdents.omitIdents && rewriteIdents.omitIdents.class || [];
132148
}
133149

134-
// TODO: scan css files for other classes in use and add them to `omitIdents`.
150+
// Add in any additional classes that were passed in using the appClasses alias.
151+
omitIdents.push(...appClassesAlias);
135152

136153
optimizerOptions.rewriteIdents = {
137154
id: false,
@@ -165,17 +182,28 @@ export class CSSBlocksStylesPreprocessorPlugin extends Plugin {
165182
this.cssBlocksOptions = cssBlocksOptions;
166183
}
167184
async build() {
168-
// Are there any changes to make? If not, bail out early.
169185
let stylesheetPath = cssBlocksPreprocessFilename(this.cssBlocksOptions);
170-
let entries = this.input.entries(".", {globs: [stylesheetPath]});
186+
187+
// Are there any changes to make? If not, bail out early.
188+
let entries = this.input.entries(
189+
".",
190+
{
191+
globs: [
192+
stylesheetPath,
193+
"app/styles/css-blocks-style-mapping.css",
194+
],
195+
},
196+
);
171197
let currentFSTree = FSTree.fromEntries(entries);
172198
let patch = this.previousSourceTree.calculatePatch(currentFSTree);
173199
if (patch.length === 0) {
174200
return;
175201
} else {
176202
this.previousSourceTree = currentFSTree;
177203
}
178-
// Read in the CSS Blocks compiled content that was created previously.
204+
205+
// Read in the CSS Blocks compiled content that was created previously
206+
// from the template tree.
179207
let blocksFileContents: string;
180208
if (this.input.existsSync(stylesheetPath)) {
181209
blocksFileContents = this.input.readFileSync(stylesheetPath, { encoding: "utf8" });
@@ -185,10 +213,16 @@ export class CSSBlocksStylesPreprocessorPlugin extends Plugin {
185213
blocksFileContents = "";
186214
}
187215

188-
// Now, write out compiled content to its expected location.
216+
// Now, write out compiled content to its expected location in the CSS tree.
189217
// By default, this is app/styles/css-blocks.css.
190218
this.output.mkdirSync(path.dirname(stylesheetPath), { recursive: true });
191219
this.output.writeFileSync(stylesheetPath, blocksFileContents);
220+
221+
// Also, forward along the JSON list of optimizer-generated class names.
222+
if (this.input.existsSync(optimizedStylesPreprocessFilepath)) {
223+
const dataContent = this.input.readFileSync(optimizedStylesPreprocessFilepath).toString("utf8");
224+
this.output.writeFileSync(optimizedStylesPreprocessFilepath, dataContent);
225+
}
192226
}
193227
}
194228

@@ -201,10 +235,12 @@ export class CSSBlocksStylesPreprocessorPlugin extends Plugin {
201235
*/
202236
export class CSSBlocksStylesPostprocessorPlugin extends Filter {
203237
env: AddonEnvironment;
238+
previousSourceTree: FSTree;
204239

205240
constructor(env: AddonEnvironment, inputNodes: InputNode[]) {
206241
super(mergeTrees(inputNodes), {});
207242
this.env = env;
243+
this.previousSourceTree = new FSTree();
208244
}
209245

210246
processString(contents: string, _relativePath: string): string {
@@ -214,28 +250,57 @@ export class CSSBlocksStylesPostprocessorPlugin extends Filter {
214250
async build() {
215251
await super.build();
216252

217-
// Look up all the application style content that's already present.
218253
const blocksCssFile = cssBlocksPostprocessFilename(this.env.config);
219-
const appStyles: string[] = [];
254+
let optimizerClasses: string[] = [];
255+
const appCss: string[] = [];
220256
const foundFiles: string[] = [];
257+
const foundClasses: Set<string> = new Set<string>();
258+
const errorLog: string[] = [];
221259

260+
// Are there any changes to make? If not, bail out early.
261+
let entries = this.input.entries(
262+
".",
263+
{
264+
globs: [
265+
"**/*.css",
266+
optimizedStylesPostprocessFilepath,
267+
],
268+
},
269+
);
270+
let currentFSTree = FSTree.fromEntries(entries);
271+
let patch = this.previousSourceTree.calculatePatch(currentFSTree);
272+
if (patch.length === 0) {
273+
return;
274+
} else {
275+
this.previousSourceTree = currentFSTree;
276+
}
277+
278+
// Read in the list of classes generated by the optimizer.
279+
if (this.input.existsSync(optimizedStylesPostprocessFilepath)) {
280+
optimizerClasses = JSON.parse(this.input.readFileSync(optimizedStylesPostprocessFilepath).toString("utf8"));
281+
} else {
282+
// Welp, nothing to do if we don't have optimizer data.
283+
debug("Skipping conflict analysis because there is no optimizer data.");
284+
return;
285+
}
286+
287+
// Look up all the application style content that's already present.
222288
const walkEntries = this.input.entries(undefined, {
223289
globs: ["**/*.css"],
224290
});
225291
walkEntries.forEach(entry => {
226292
if (entry.relativePath === blocksCssFile) return;
227293
try {
228-
appStyles.push(this.input.readFileSync(entry.relativePath).toString("utf8"));
294+
appCss.push(this.input.readFileSync(entry.relativePath).toString("utf8"));
229295
foundFiles.push(entry.relativePath);
230296
} catch (e) {
231297
// broccoli-concat will complain about this later. let's move on.
232298
}
233299
});
300+
debug("Done looking up app CSS.");
234301

235-
// Now, read in each of these sources and check there are no class name conflicts.
236-
const foundClasses: Set<string> = new Set<string>();
237-
const errorLog: string[] = [];
238-
appStyles.forEach(content => {
302+
// Now, read in each of these sources and note all classes found.
303+
appCss.forEach(content => {
239304
try {
240305
const parsed = postcss.parse(content);
241306
parsed.walkRules(rule => {
@@ -249,19 +314,38 @@ export class CSSBlocksStylesPostprocessorPlugin extends Filter {
249314
});
250315
});
251316
} catch (e) {
252-
// TODO: Can't parse CSS? Gracefully fail or crash and burn?
317+
// Can't parse CSS? We'll skip it and add a warning to the log.
253318
errorLog.push(e.toString());
319+
debug(`Ran into an error when parsing CSS content for conflict analysis! Review the error log for details.`);
254320
}
255321
});
322+
debug("Done finding app classes.");
323+
324+
// Find collisions between the app styles and optimizer styles.
325+
const collisions = optimizerClasses.filter(val => foundClasses.has(val));
326+
debug("Done identifying collisions.");
256327

257328
// Build a logfile for the output tree, for debugging.
258-
let logfile = "FOUND CLASSES:\n";
329+
let logfile = "COLLISIONS:\n";
330+
collisions.forEach(cssClass => { logfile += `${cssClass}\n`; });
331+
logfile += "\nFOUND APP CLASSES:\n";
259332
foundClasses.forEach(cssClass => { logfile += `${cssClass}\n`; });
260333
logfile += "\nFOUND FILES:\n";
261334
foundFiles.forEach(file => { logfile += `${file}\n`; });
262335
logfile += "\nERRORS:\n";
263336
errorLog.forEach(err => { logfile += `${err}\n`; });
264337
this.output.writeFileSync("assets/app-classes.log", logfile);
338+
debug("Wrote log file to broccoli tree.");
339+
340+
if (collisions.length > 0) {
341+
throw new Error(
342+
"Your application CSS contains classes that are also generated by the CSS optimizer. This can cause style conflicts between your application's classes and those generated by CSS Blocks.\n" +
343+
"To resolve this conflict, you should add any short class names in non-block CSS (~5 characters or less) to the list of disallowed classes in your build configuration.\n" +
344+
"(You can do this by setting css-blocks.appClasses to an array of disallowed classes in ember-cli-build.js.)\n\n" +
345+
"Conflicting classes:\n" +
346+
collisions.reduce((prev, curr) => prev += `${curr}\n`, ""),
347+
);
348+
}
265349
}
266350
}
267351

packages/@css-blocks/ember-app/src/index.ts

+12-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import type { AddonImplementation, ThisAddon } from "ember-cli/lib/models/addon"
99
import Project from "ember-cli/lib/models/project";
1010

1111
import { CSSBlocksApplicationPlugin, CSSBlocksStylesPostprocessorPlugin, CSSBlocksStylesPreprocessorPlugin } from "./broccoli-plugin";
12-
import { appStylesPostprocessFilename, cssBlocksPostprocessFilename } from "./utils/filepaths";
12+
import { appStylesPostprocessFilename, cssBlocksPostprocessFilename, optimizedStylesPostprocessFilepath } from "./utils/filepaths";
1313
import { AddonEnvironment, CSSBlocksApplicationAddon } from "./utils/interfaces";
1414

1515
/**
@@ -172,14 +172,20 @@ const EMBER_ADDON: AddonImplementation<CSSBlocksApplicationAddon> = {
172172
// tree is processed before the CSS tree, but just in case....
173173
throw new Error("[css-blocks/ember-app] The CSS tree ran before the JS tree, so the CSS tree doesn't have the contents for CSS Blocks files. This shouldn't ever happen, but if it does, please file an issue with us!");
174174
}
175-
// Get the combined CSS file
175+
// Copy over the CSS Blocks compiled output from the template tree to the CSS tree.
176176
const cssBlocksContentsTree = new CSSBlocksStylesPreprocessorPlugin(env.modulePrefix, env.config, [this.broccoliAppPluginInstance, tree]);
177177
return new BroccoliDebug(mergeTrees([tree, cssBlocksContentsTree], { overwrite: true }), "css-blocks:css-preprocess");
178178
} else {
179179
return tree;
180180
}
181181
},
182182

183+
/**
184+
* Post-process the tree.
185+
* @param type - The type of tree.
186+
* @param tree - The tree to process.
187+
* @returns - A broccoli tree.
188+
*/
183189
postprocessTree(type, tree) {
184190
let env = this.env!;
185191

@@ -208,7 +214,10 @@ const EMBER_ADDON: AddonImplementation<CSSBlocksApplicationAddon> = {
208214

209215
// Then overwrite the original file with our final build artifact.
210216
const mergedTree = funnel(mergeTrees([tree, concatTree], { overwrite: true }), {
211-
exclude: [cssBlocksPostprocessFilename(env.config)],
217+
exclude: [
218+
cssBlocksPostprocessFilename(env.config),
219+
optimizedStylesPostprocessFilepath,
220+
],
212221
});
213222
return new BroccoliDebug(mergedTree, "css-blocks:css-postprocess");
214223
}

packages/@css-blocks/ember-app/src/utils/filepaths.ts

+10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@ import { ResolvedCSSBlocksEmberOptions } from "@css-blocks/ember-utils";
22

33
import { AddonEnvironment } from "./interfaces";
44

5+
/**
6+
* Filepath for list of optimized styles in preprocess tree.
7+
*/
8+
export const optimizedStylesPreprocessFilepath = "app/styles/css-blocks-stylelist.json";
9+
10+
/**
11+
* Filepath for list of optimized styles in postprocess tree.
12+
*/
13+
export const optimizedStylesPostprocessFilepath = "assets/css-blocks-stylelist.json";
14+
515
/**
616
* Generate the output path for the compiled CSS Blocks content, using the
717
* preferred filename given by the user. If none is given, the default

packages/@css-blocks/ember-utils/src/options.ts

+25
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,42 @@ export interface BroccoliConcatOptions {
4949
}
5050

5151
export interface CSSBlocksEmberOptions {
52+
/**
53+
* The name of the CSS file generated that contains the compiled
54+
* result of CSS Blocks. By default, this is css-blocks.css.
55+
*/
5256
output?: string;
5357
aliases?: ObjectDictionary<string>;
5458
analysisOpts?: AnalysisOptions;
5559
parserOpts?: Writeable<ParserOptions>;
60+
/**
61+
* Options that control the behavior of OptiCSS, which is used to
62+
* rewrite and optimize compiled CSS Blocks output. By default,
63+
* optimization is enabled for production builds only.
64+
*
65+
* To disable optimization, set optimization.enabled to false.
66+
*/
5667
optimization?: Partial<OptiCSSOptions>;
5768
/**
5869
* Options that control the behavior of broccoli-concat, which is used
5970
* to concatenate CSS files together by ember-app during postprocess.
71+
*
6072
* If this is set to false, broccoli-concat will *not* run.
6173
* You'll need to add additional processing to add the CSS Blocks
6274
* compiled content to your final CSS build artifact.
6375
*/
6476
broccoliConcat?: BroccoliConcatOptions | false;
77+
/**
78+
* List of classes that are used by application CSS and might conflict
79+
* with the optimizer. You should add any short class names (~5 characters)
80+
* to this list so the optimizer doesn't use these when building the
81+
* CSS Blocks compiled output.
82+
*
83+
* This is a convenience alias for
84+
* optimization.rewriteIdents.omitIdents.class[]. It has no effect if
85+
* optimization is disabled.
86+
*/
87+
appClasses?: string[];
6588
}
6689

6790
export interface ResolvedCSSBlocksEmberOptions {
@@ -71,13 +94,15 @@ export interface ResolvedCSSBlocksEmberOptions {
7194
parserOpts: ParserOptions;
7295
optimization: Partial<OptiCSSOptions>;
7396
broccoliConcat: BroccoliConcatOptions | false;
97+
appClasses: string[];
7498
}
7599

76100
export function getConfig(root: string, isProduction: boolean, options: CSSBlocksEmberOptions): ResolvedCSSBlocksEmberOptions {
77101
if (!options.aliases) options.aliases = {};
78102
if (!options.analysisOpts) options.analysisOpts = {};
79103
if (!options.optimization) options.optimization = {};
80104
if (!options.broccoliConcat) options.broccoliConcat = {};
105+
if (!options.appClasses) options.appClasses = [];
81106

82107
if (!options.parserOpts) {
83108
options.parserOpts = config.searchSync(root) || {};

private-packages/fixtures-ember-v2/ember-app/app/styles/app.css

+19
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,23 @@ body {
1010

1111
.public-addon-component-alias {
1212
font-size: 22px;
13+
}
14+
15+
/*
16+
* This class is a smoke test for the class duplication check run by
17+
* the @css-blocks/ember-app css postprocess step. When optimization is
18+
* enabled, this class conflicts with the default classes that are generated
19+
* by OptiCSS.
20+
*
21+
* You can force a build failure by commenting out css-blocks.appStyles in
22+
* ember-cli-build.js and building using the --prod flag (or turning on
23+
* optimization via config).
24+
*
25+
* (Note: This info is accurate as of the time of this comment's creation.
26+
* Other changes may make it so .i no longer conflicts with optimization. In
27+
* this case, review the generated css-blocks output for a class name that
28+
* conflicts and change this to use that class name.)
29+
*/
30+
.i {
31+
font-style: italic;
1332
}

private-packages/fixtures-ember-v2/ember-app/ember-cli-build.js

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ module.exports = function(defaults) {
1212
mergeDeclarations: true,
1313
removeUnusedStyles: true,
1414
},
15+
appClasses: ['i'],
1516
}
1617
});
1718

0 commit comments

Comments
 (0)