Skip to content

Commit

Permalink
fix(opticss): numerous cascade violations
Browse files Browse the repository at this point in the history
  • Loading branch information
@chriseppstein
chriseppstein committed Oct 15, 2017
1 parent 48ef1f7 commit c6a0b13
Show file tree
Hide file tree
Showing 8 changed files with 430 additions and 86 deletions.
20 changes: 7 additions & 13 deletions packages/opticss/src/actions/MergeDeclarations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
import {
MultiAction,
} from './Action';
import { DeclarationInfo } from '../optimizations/MergeDeclarations/StyleInfo';

const REWRITEABLE_ATTR_OPS = ["=", "~=", undefined];

Expand All @@ -49,13 +50,6 @@ export interface Declaration {
important: boolean;
}

export interface DeclarationInfo {
selector: ParsedSelector;
rule: postcss.Rule;
decl: postcss.Declaration;
container: postcss.Node;
}

/**
* Merges duplicate declarations from multiple rule sets into a new rule set.
*/
Expand All @@ -77,7 +71,6 @@ export class MergeDeclarations extends MultiAction {
constructor(
templateOptions: TemplateIntegrationOptions,
pass: OptimizationPass,
container: postcss.Container,
selectorContext: ParsedSelector | undefined,
decl: Declaration,
declInfos: Array<DeclarationInfo>,
Expand All @@ -88,7 +81,7 @@ export class MergeDeclarations extends MultiAction {
this.templateOptions = templateOptions;
this.styleMapping = pass.styleMapping;
this.reason = reason;
this.container = container;
this.container = declInfos[0].selectorInfo.container;
this.selectorContext = selectorContext;
this.cache = pass.cache;
this.identGenerators = pass.identGenerators;
Expand Down Expand Up @@ -117,11 +110,12 @@ export class MergeDeclarations extends MultiAction {
decl.raws = { before:' ', after: ' '};
this.newRule.append(decl);

let ruleLocation = this.declInfos.find(d => d.container === this.container)!.rule;
let insertionDecl = this.declInfos[0];
let ruleLocation = insertionDecl.selectorInfo.rule;
this.container.insertBefore(ruleLocation, this.newRule);
let sourceAttributes = new Array<ElementAttributes>();
for (let declInfo of this.declInfos) {
let sel: CompoundSelector = declInfo.selector.key;
let sel: CompoundSelector = declInfo.selectorInfo.selector.key;
let inputs = inputsFromSelector(this.templateOptions, sel);
if (!inputs) {
throw new Error("internal error");
Expand All @@ -131,7 +125,7 @@ export class MergeDeclarations extends MultiAction {
unless: new Array<ElementTagname | ElementAttribute>() // TODO: cascade resolution of exclusion classes.
});
if (declInfo.decl.parent === undefined) {
continue;
continue; // TODO: take this out -- it shouldn't happen.
}
if (declInfo.decl.parent.nodes!.filter(node => node.type === "decl").length === 1) {
let rule = <postcss.Rule>declInfo.decl.parent;
Expand Down Expand Up @@ -161,7 +155,7 @@ export class MergeDeclarations extends MultiAction {
logStrings(): Array<string> {
let logs = new Array<string>();
this.declInfos.forEach((orig, i) => {
let msg = `Declaration moved from "${orig.selector}" into generated rule (${this.declString()}). ${this.reason} ${i + 1} of ${this.declInfos.length}.`;
let msg = `Declaration moved from "${orig.selectorInfo.selector}" into generated rule (${this.declString()}). ${this.reason} ${i + 1} of ${this.declInfos.length}.`;
logs.push(this.annotateLogMessage(msg, this.nodeSourcePosition(orig.decl)));
});
this.removedRules.forEach(rule => {
Expand Down
30 changes: 17 additions & 13 deletions packages/opticss/src/optimizations/MergeDeclarations/DeclarationMapper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { ParsedCssFile } from '../../CssFile';
import { OptimizationPass } from '../../OptimizationPass';
import {
expandIfNecessary,
expandPropertyName,
} from '../../util/shorthandProperties';
import { walkRules } from '../util';
import { SelectorInfo, DeclarationInfo } from './StyleInfo';
Expand Down Expand Up @@ -56,6 +55,8 @@ export class DeclarationMapper {
if (cmp === 0) cmp = compare(s1.sourceIndex, s2.sourceIndex);
return cmp;
});
let declSourceIndex = 0;
let declSourceIndexMap = new Map<postcss.Declaration, number>();
this.elementDeclarations = new Map<Element, MultiDictionary<string, DeclarationInfo>>();
files.forEach((file, fileIndex) => {
let sourceIndex = 0;
Expand All @@ -64,6 +65,7 @@ export class DeclarationMapper {
/** all the declarations of this rule after expanding longhand properties. */
let declarations = new MultiDictionary<string,[string, boolean, postcss.Declaration]>(undefined, undefined, true);
rule.walkDecls(decl => {
declSourceIndexMap.set(decl, declSourceIndex++);
// TODO: normalize values. E.g colors of different formats, etc.
declarations.setValue(decl.prop, [decl.value, decl.important, decl]);
});
Expand Down Expand Up @@ -104,9 +106,12 @@ export class DeclarationMapper {

// map properties to selector info
let context = this.contexts.getContext(selectorInfo.rule.root(), selectorInfo.scope, selectorInfo.selector.toContext());
selectorInfo.declarations.keys().forEach(prop => {
for (let prop of selectorInfo.declarations.keys()) {
if (/\.gb_fa/.test(selectorInfo.rule.selector) && /border/.test(prop)) {
console.log(selectorInfo.selector.toString());
}
let values = selectorInfo.declarations.getValue(prop);
values.forEach(value => {
for (let value of values) {
declarationOrdinal++;
let [v, important, decl] = value;
if (propParser.isShorthandProperty(decl.prop)) {
Expand All @@ -117,25 +122,20 @@ export class DeclarationMapper {
let longHandProps = Object.keys(longhandDeclarations);
let longHandDeclInfos = new Array<DeclarationInfo>();
for (let longHandProp of longHandProps) {
let declInfo = this.makeDeclInfo(selectorInfo, longHandProp, longhandDeclarations[longHandProp], important, decl, declarationOrdinal);
let declInfo = this.makeDeclInfo(selectorInfo, longHandProp, longhandDeclarations[longHandProp], important, decl, declSourceIndexMap.get(decl)!, declarationOrdinal);
longHandDeclInfos.push(declInfo);
if (important) {
importantDeclInfos.push(declInfo);
}
selectorInfo.declarationInfos.setValue([prop, v], declInfo);
let valueInfo = context.getDeclarationValues(longHandProp);
valueInfo.setValue(longhandDeclarations[longHandProp], declInfo);
if (propParser.isShorthandProperty(longHandProp)) {
let allDecls = expandIfNecessary(new Set(expandPropertyName(longHandProp, true)), longHandProp, longhandDeclarations[longHandProp]);
for (let longHandProp of Object.keys(allDecls)) {
this.addDeclInfoToElements(selectorInfo.elements, longHandProp, declInfo);
}
}
this.addDeclInfoToElements(selectorInfo.elements, longHandProp, declInfo);
}
this.trackDeclarationInfo(context, longHandDeclInfos);
} else {
// normal long hand props are just set directly
let declInfo = this.makeDeclInfo(selectorInfo, prop, v, important, decl, declarationOrdinal);
let declInfo = this.makeDeclInfo(selectorInfo, prop, v, important, decl, declSourceIndexMap.get(decl)!, declarationOrdinal);
this.trackDeclarationInfo(context, [declInfo]);
if (important) {
importantDeclInfos.push(declInfo);
Expand All @@ -144,8 +144,8 @@ export class DeclarationMapper {
valueInfo.setValue(v, declInfo);
this.addDeclInfoToElements(selectorInfo.elements, prop, declInfo);
}
});
});
}
}
});
// we add the max declaration ordinal to all the important declaration infos
// this makes those declarations resolve higher than all the non-important values.
Expand All @@ -159,6 +159,7 @@ export class DeclarationMapper {
value: string,
important: boolean,
decl: postcss.Declaration,
sourceOrdinal: number,
ordinal: number,
dupeCount = 0
): DeclarationInfo {
Expand All @@ -168,7 +169,10 @@ export class DeclarationMapper {
value,
important,
selectorInfo,
sourceOrdinal,
originalSourceOrdinal: sourceOrdinal,
ordinal,
originalOrdinal: ordinal,
dupeCount,
merged: false,
expanded: false,
Expand Down
7 changes: 7 additions & 0 deletions packages/opticss/src/optimizations/MergeDeclarations/OptimizationContext.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import * as postcss from 'postcss';
import { Dictionary, MultiDictionary } from 'typescript-collections';
import * as specificity from 'specificity';
import * as selectorParser from 'postcss-selector-parser';

import { ParsedSelector } from '../../parseSelector';
import { RuleScope } from '../util';
Expand All @@ -17,6 +19,7 @@ export class OptimizationContext {
/** runtime selector scoping for this context. */
selectorContext: ParsedSelector | undefined;
root: postcss.Root;
specificity: specificity.Specificity | undefined;

/**
* map of property keys to a dictionary of values multi-mapped to
Expand All @@ -43,6 +46,10 @@ export class OptimizationContext {
this.declarationMap = new Dictionary<string, MultiDictionary<string, DeclarationInfo>>();
this.authoredProps = new Set();
this.declarationInfos = new Set();
let specificitySelector = selectorContext &&
selectorContext.toContext(selectorParser.className({ value: "foo" }));
this.specificity = specificitySelector &&
specificity.calculate(specificitySelector.toString())[0];
}

getDeclarationValues(prop: string): MultiDictionary<string, DeclarationInfo> {
Expand Down
13 changes: 12 additions & 1 deletion packages/opticss/src/optimizations/MergeDeclarations/StyleInfo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export interface SelectorInfo {
scope: RuleScope;
/** The selector parsed into compound selectors */
selector: ParsedSelector;
container: postcss.Node;
container: postcss.Container;
/** The specificity of this selector */
specificity: specificity.Specificity;
/** The file this selector came from */
Expand Down Expand Up @@ -57,12 +57,23 @@ export interface DeclarationInfo {
prop: string;
value: string;
important: boolean;
/**
* A single number that can be compared with another DeclarationInfo
* to understand which one came first in the document source order.
* Smaller numbers are closer to the start of the file.
*/
sourceOrdinal: number;
/** the original source ordinal value before the declaration was moved. */
originalSourceOrdinal: number;
/**
* A single number that can be compared with another DeclarationInfo
* to understand which one wins if both are applied on the same element.
* Bigger numbers win.
*/
ordinal: number;
/** the original ordinal value before the declaration was moved. */
originalOrdinal: number;
/** How many declarations this one can be merged with. */
dupeCount: number;
/** whether this decl has been expanded yet. */
expanded: boolean;
Expand Down

0 comments on commit c6a0b13

Please sign in to comment.