Skip to content

Commit c70ed03

Browse files
amiller-ghamiller-gh
committed
fix: Over-zealous conflicts from inherited in-stylesheet compositions.
1 parent b44f68e commit c70ed03

File tree

5 files changed

+86
-49
lines changed

5 files changed

+86
-49
lines changed

‎packages/@css-blocks/core/src/Analyzer/validations/property-conflict-validator.ts

+29-13
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ function evaluate(obj: Style, propToRules: PropMap, conflicts: ConflictMap) {
5656
// If these styles are from the same block, abort!
5757
if (other.style.block === self.style.block) { continue; }
5858

59+
// If these styles are descendants, abort! This may happen from
60+
// in-stylesheet composition.
61+
if (
62+
other.style.block.isAncestorOf(self.style.block) ||
63+
self.style.block.isAncestorOf(other.style.block)
64+
) { continue; }
65+
66+
// If one style composes the other somewhere in its hierarchy, abort!
67+
// This will have been resolved on the base Block, No need to resolve.
68+
if (
69+
isBlockClass(other.style) && !other.style.composes(self.style, false) && other.style.composes(self.style) ||
70+
isBlockClass(self.style) && !self.style.composes(other.style, false) && self.style.composes(other.style)
71+
) { continue; }
72+
5973
// Get the declarations for this specific property.
6074
let selfDecl = self.declarations.get(prop);
6175
let otherDecl = other.declarations.get(prop);
@@ -71,9 +85,9 @@ function evaluate(obj: Style, propToRules: PropMap, conflicts: ConflictMap) {
7185
}
7286
}
7387
if (valuesEqual ||
74-
other.hasResolutionFor(prop, self.style) ||
75-
self.hasResolutionFor(prop, other.style)
76-
) { continue; }
88+
other.hasResolutionFor(prop, self.style) ||
89+
self.hasResolutionFor(prop, other.style)
90+
) { continue; }
7791

7892
// Otherwise, we found an unresolved conflict!
7993
conflicts.set(prop, other);
@@ -111,16 +125,18 @@ function recursivelyPruneConflicts(prop: string, conflicts: ConflictMap): Rulese
111125
* @param prop The property we're printing on this Ruleset.
112126
* @param rule The Ruleset we're printing.
113127
*/
114-
function printRulesetConflict(prop: string, rule: Ruleset) {
115-
let decl = rule.declarations.get(prop);
116-
let nodes: postcss.Rule[] | postcss.Declaration[] = decl ? decl.map((d) => d.node) : [rule.node];
117-
let out = [];
118-
for (let node of nodes) {
119-
let line = node.source.start && `:${node.source.start.line}`;
120-
let column = node.source.start && `:${node.source.start.column}`;
121-
out.push(` ${rule.style.asSource(true)} (${rule.file}${line}${column})`);
128+
function printRulesetConflict(prop: string, rules: Ruleset<Style>[]) {
129+
const out = new Set();
130+
for (let rule of rules) {
131+
let decl = rule.declarations.get(prop);
132+
let nodes: postcss.Rule[] | postcss.Declaration[] = decl ? decl.map((d) => d.node) : [rule.node];
133+
for (let node of nodes) {
134+
let line = node.source.start && `:${node.source.start.line}`;
135+
let column = node.source.start && `:${node.source.start.column}`;
136+
out.add(` ${rule.style.asSource(true)} (${rule.file}${line}${column})`);
137+
}
122138
}
123-
return out.join("\n");
139+
return [...out].join("\n");
124140
}
125141

126142
function inStylesheetComposition(
@@ -220,7 +236,7 @@ export const propertyConflictValidator: Validator = (elAnalysis, _templateAnalys
220236
let details = "\n";
221237
for (let [prop, matches] of conflicts.entries()) {
222238
if (!prop || !matches.length) { return; }
223-
details += ` ${prop}:\n${matches.map((m) => printRulesetConflict(prop, m)).join("\n")}\n\n`;
239+
details += ` ${prop}:\n${printRulesetConflict(prop, matches)}\n\n`;
224240
}
225241
err(msg, null, details);
226242
}

‎packages/@css-blocks/core/src/BlockTree/BlockClass.ts

+15
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,21 @@ export class BlockClass extends Style<BlockClass, Block, Block, Attribute> {
268268
this._composedStyles.add({ style, conditions });
269269
}
270270

271+
/**
272+
* Adds a new Style for this Style to compose.
273+
* TODO: Currently, conditions are grouped exclusively by the 'and' operator.
274+
* We can abstract boolean operators to keep an internal representation
275+
* of logic between css and template files and only resolve them to the
276+
* requested language interface at rewrite time.
277+
*/
278+
composes(style: Styles, resolve = true): boolean {
279+
let compositions = resolve ? this.resolveComposedStyles() : new Set(this._composedStyles);
280+
for (let comp of compositions) {
281+
if (comp.style === style) { return true; }
282+
}
283+
return false;
284+
}
285+
271286
/**
272287
* Debug utility to help test BlockClasses.
273288
* @param options Options to pass to BlockClass' asDebug method.

‎packages/@css-blocks/core/src/importing/NodeJsImporter.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export class NodeJsImporter implements Importer {
3434
// Normalize aliases input.
3535
this.aliases = Array.isArray(aliases)
3636
? aliases.slice()
37-
: Object.keys(aliases).map(alias => ({ alias: alias, path: aliases[alias] }));
37+
: Object.keys(aliases).map(alias => ({ alias, path: aliases[alias] }));
3838

3939
// Sort aliases most specific to least specific.
4040
this.aliases.sort((a, b) => b.path.length - a.path.length);
@@ -51,20 +51,22 @@ export class NodeJsImporter implements Importer {
5151
// If absolute, this is the identifier.
5252
if (path.isAbsolute(importPath)) { return importPath; }
5353

54-
// Attempt to resolve to absolute path relative to `from` or `rootDir`.
55-
// If it exists, return.
54+
// Attempt to resolve relative path to absolute path relative to the
55+
// `from` or `rootDir`. If it exists, return.
5656
from = from ? this.filesystemPath(from, config) : from;
5757
let fromDir = from ? path.dirname(from) : config.rootDir;
5858
let resolvedPath = path.resolve(fromDir, importPath);
5959
if (existsSync(resolvedPath)) { return resolvedPath; }
6060
debug(`No relative or absolute Block file discovered for ${importPath}.`);
6161

62-
// If not a real file, attempt to resolve to an aliased path instead.
63-
let alias = this.aliases.find(a => importPath.startsWith(a.alias + path.sep));
62+
// If not a real file, attempt to resolve to an aliased path instead, if present.
63+
let alias = this.aliases.find(a => importPath.startsWith(a.alias) || importPath.startsWith(a.alias + path.sep));
6464
if (alias) {
65-
return path.resolve(alias.path, importPath.substring(alias.alias.length + 1));
65+
importPath = path.join(alias.path, importPath.replace(alias.alias, ""));
66+
}
67+
else {
68+
debug(`No file path alias discovered for ${importPath}.`);
6669
}
67-
debug(`No file path alias discovered for ${importPath}.`);
6870

6971
// If no alias found, test for a node_module resolution as a file path.
7072
try {

‎packages/@css-blocks/ember-cli/index.js

+17-6
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
'use strict';
2-
const fs = require("fs");
32
const path = require("path");
43

54
const { CSSBlocksAggregate, CSSBlocksAnalyze, Transport } = require("@css-blocks/broccoli");
65
const { GlimmerAnalyzer, GlimmerRewriter } = require("@css-blocks/glimmer");
6+
const { NodeJsImporter } = require("@css-blocks/core");
77

8-
const BroccoliStew = require("broccoli-stew");
98
const BroccoliConcat = require("broccoli-concat");
109
const BroccoliMerge = require("broccoli-merge-trees");
1110

@@ -23,7 +22,12 @@ const NOOP = (tree) => tree;
2322

2423
// Default no-op plugin for templates with no associated CSS Block.
2524
// `visitors` is used by Ember < 3.0.0. `visitor` is used by Glimmer and Ember >= 3.0.0.
26-
const NOOP_PLUGIN = { name: 'css-blocks-noop', visitors: {}, visitor: {} };
25+
const NOOP_PLUGIN = {
26+
name: 'css-blocks-noop',
27+
visitors: {},
28+
visitor: {},
29+
cacheKey: () => { return 1; }
30+
};
2731

2832
module.exports = {
2933
name: '@css-blocks/ember-cli',
@@ -254,11 +258,18 @@ module.exports = {
254258
const options = app.options["css-blocks"]
255259
? app.options["css-blocks"] // Do not clone! Contains non-json-safe data.
256260
: {
261+
aliases: {},
257262
parserOpts: {},
258263
analysisOpts: {},
259264
optimization: {},
260265
};
261-
options.parserOpts || (options.parserOpts = {});
266+
options.aliases || (options.aliases = {});
267+
268+
// if (!options.aliases[])
269+
270+
options.parserOpts || (options.parserOpts = {
271+
importer: new NodeJsImporter(options.aliases),
272+
});
262273
options.analysisOpts || (options.analysisOpts = {});
263274
options.optimization || (options.optimization = {});
264275

@@ -299,11 +310,11 @@ module.exports = {
299310
analyzer.transport = transport;
300311

301312
const broccoliOptions = {
302-
entry,
303313
analyzer,
304-
root: rootDir,
314+
entry,
305315
output: options.output,
306316
optimization: options.optimization,
317+
root: rootDir,
307318
};
308319

309320
return (tree) => {

‎yarn.lock

+16-23
Original file line numberDiff line numberDiff line change
@@ -2336,7 +2336,7 @@ axobject-query@^0.1.0:
23362336
dependencies:
23372337
ast-types-flow "0.0.7"
23382338

2339-
[email protected], babel-code-frame@^6.11.0, babel-code-frame@^6.22.0, babel-code-frame@^6.26.0:
2339+
[email protected], babel-code-frame@^6.22.0, babel-code-frame@^6.26.0:
23402340
version "6.26.0"
23412341
resolved "https://registry.npmjs.org/babel-code-frame/-/babel-code-frame-6.26.0.tgz#63fd43f7dc1e3bb7ce35947db8fe369a3f58c74b"
23422342
dependencies:
@@ -5024,22 +5024,21 @@ [email protected]:
50245024
version "0.0.4"
50255025
resolved "https://registry.npmjs.org/css-color-names/-/css-color-names-0.0.4.tgz#808adc2e79cf84738069b646cb20ec27beb629e0"
50265026

5027-
[email protected]:
5028-
version "0.28.7"
5029-
resolved "https://registry.npmjs.org/css-loader/-/css-loader-0.28.7.tgz#5f2ee989dd32edd907717f953317656160999c1b"
5027+
[email protected]:
5028+
version "1.0.0"
5029+
resolved "https://registry.npmjs.org/css-loader/-/css-loader-1.0.0.tgz#9f46aaa5ca41dbe31860e3b62b8e23c42916bf56"
5030+
integrity sha512-tMXlTYf3mIMt3b0dDCOQFJiVvxbocJ5Ho577WiGPYPZcqVEO218L2iU22pDXzkTZCLDE+9AmGSUkWxeh/nZReA==
50305031
dependencies:
5031-
babel-code-frame "^6.11.0"
5032+
babel-code-frame "^6.26.0"
50325033
css-selector-tokenizer "^0.7.0"
5033-
cssnano ">=2.6.1 <4"
50345034
icss-utils "^2.1.0"
50355035
loader-utils "^1.0.2"
50365036
lodash.camelcase "^4.3.0"
5037-
object-assign "^4.0.1"
5038-
postcss "^5.0.6"
5039-
postcss-modules-extract-imports "^1.0.0"
5040-
postcss-modules-local-by-default "^1.0.1"
5041-
postcss-modules-scope "^1.0.0"
5042-
postcss-modules-values "^1.1.0"
5037+
postcss "^6.0.23"
5038+
postcss-modules-extract-imports "^1.2.0"
5039+
postcss-modules-local-by-default "^1.2.0"
5040+
postcss-modules-scope "^1.1.0"
5041+
postcss-modules-values "^1.3.0"
50435042
postcss-value-parser "^3.3.0"
50445043
source-list-map "^2.0.0"
50455044

@@ -5099,7 +5098,7 @@ cssesc@^1.0.1:
50995098
version "1.0.1"
51005099
resolved "https://registry.npmjs.org/cssesc/-/cssesc-1.0.1.tgz#ef7bd8d0229ed6a3a7051ff7771265fe7330e0a8"
51015100

5102-
"cssnano@>=2.6.1 <4", cssnano@^3.10.0:
5101+
cssnano@^3.10.0:
51035102
version "3.10.0"
51045103
resolved "https://registry.npmjs.org/cssnano/-/cssnano-3.10.0.tgz#4f38f6cea2b9b17fa01490f23f1dc68ea65c1c38"
51055104
dependencies:
@@ -11447,33 +11446,27 @@ postcss-minify-selectors@^2.0.4:
1144711446
postcss "^5.0.14"
1144811447
postcss-selector-parser "^2.0.0"
1144911448

11450-
postcss-modules-extract-imports@^1.0.0:
11451-
version "1.1.0"
11452-
resolved "https://registry.npmjs.org/postcss-modules-extract-imports/-/postcss-modules-extract-imports-1.1.0.tgz#b614c9720be6816eaee35fb3a5faa1dba6a05ddb"
11453-
dependencies:
11454-
postcss "^6.0.1"
11455-
1145611449
postcss-modules-extract-imports@^1.2.0:
1145711450
version "1.2.0"
1145811451
resolved "https://registry.npmjs.org/postcss-modules-extract-imports/-/postcss-modules-extract-imports-1.2.0.tgz#66140ecece38ef06bf0d3e355d69bf59d141ea85"
1145911452
dependencies:
1146011453
postcss "^6.0.1"
1146111454

11462-
postcss-modules-local-by-default@^1.0.1, postcss-modules-local-by-default@^1.2.0:
11455+
postcss-modules-local-by-default@^1.2.0:
1146311456
version "1.2.0"
1146411457
resolved "https://registry.npmjs.org/postcss-modules-local-by-default/-/postcss-modules-local-by-default-1.2.0.tgz#f7d80c398c5a393fa7964466bd19500a7d61c069"
1146511458
dependencies:
1146611459
css-selector-tokenizer "^0.7.0"
1146711460
postcss "^6.0.1"
1146811461

11469-
postcss-modules-scope@^1.0.0, postcss-modules-scope@^1.1.0:
11462+
postcss-modules-scope@^1.1.0:
1147011463
version "1.1.0"
1147111464
resolved "https://registry.npmjs.org/postcss-modules-scope/-/postcss-modules-scope-1.1.0.tgz#d6ea64994c79f97b62a72b426fbe6056a194bb90"
1147211465
dependencies:
1147311466
css-selector-tokenizer "^0.7.0"
1147411467
postcss "^6.0.1"
1147511468

11476-
postcss-modules-values@^1.1.0, postcss-modules-values@^1.3.0:
11469+
postcss-modules-values@^1.3.0:
1147711470
version "1.3.0"
1147811471
resolved "https://registry.npmjs.org/postcss-modules-values/-/postcss-modules-values-1.3.0.tgz#ecffa9d7e192518389f42ad0e83f72aec456ea20"
1147911472
dependencies:
@@ -11583,7 +11576,7 @@ postcss@^5.0.10, postcss@^5.0.11, postcss@^5.0.12, postcss@^5.0.13, postcss@^5.0
1158311576
source-map "^0.5.6"
1158411577
supports-color "^3.2.3"
1158511578

11586-
postcss@^6.0.0, postcss@^6.0.1, postcss@^6.0.13, postcss@^6.0.21:
11579+
postcss@^6.0.0, postcss@^6.0.1, postcss@^6.0.13, postcss@^6.0.21, postcss@^6.0.23:
1158711580
version "6.0.23"
1158811581
resolved "https://registry.npmjs.org/postcss/-/postcss-6.0.23.tgz#61c82cc328ac60e677645f979054eb98bc0e3324"
1158911582
dependencies:

0 commit comments

Comments
 (0)