Skip to content

Commit

Permalink
Use insertRule to inject styles
Browse files Browse the repository at this point in the history 
Instead of appeding a new text node to a style tag or appending to the
`cssText` property of the stylesheet (which broke media queries in IE as
documented in #238), use `insertRule` to add CSS rules to the `style`
element.

This change requires styles to be generated as an array of strings with
one rule per string, rather than a single large string that contains
multiple rules.
  • Loading branch information
@reklawnos @lencioni
reklawnos authored and lencioni committed Jan 31, 2018
1 parent a057827 commit 55ed90a
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 99 deletions.
31 changes: 19 additions & 12 deletions src/generate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export type SelectorHandler = (
selector: string,
baseSelector: string,
callback: SelectorCallback
) => string | null;
) => string[] | null;
*/

/**
Expand Down Expand Up @@ -67,7 +67,7 @@ export type SelectorHandler = (
* @param {function} generateSubtreeStyles: A function which can be called to
* generate CSS for the subtree of styles corresponding to the selector.
* Accepts a new baseSelector to use for generating those styles.
* @returns {?string} The generated CSS for this selector, or null if we don't
* @returns {?string[]} The generated CSS for this selector, or null if we don't
* handle this selector.
*/
export const defaultSelectorHandlers = [
Expand All @@ -94,7 +94,7 @@ export const defaultSelectorHandlers = [
}
// Generate the styles normally, and then wrap them in the media query.
const generated = generateSubtreeStyles(baseSelector);
return `${selector}{${generated}}`;
return [`${selector}{${generated}}`];
},
];

Expand Down Expand Up @@ -147,15 +147,15 @@ export const generateCSS = (
selectorHandlers /* : SelectorHandler[] */,
stringHandlers /* : StringHandlers */,
useImportant /* : boolean */
) /* : string */ => {
) /* : string[] */ => {
const merged = new OrderedElements();

for (let i = 0; i < styleTypes.length; i++) {
merged.addStyleType(styleTypes[i]);
}

const plainDeclarations = new OrderedElements();
let generatedStyles = "";
const generatedStyles = [];

// TODO(emily): benchmark this to see if a plain for loop would be faster.
merged.forEach((val, key) => {
Expand All @@ -170,7 +170,7 @@ export const generateCSS = (
if (result != null) {
// If the handler returned something, add it to the generated
// CSS and stop looking for another handler.
generatedStyles += result;
generatedStyles.push(...result);
return true;
}
});
Expand All @@ -180,13 +180,20 @@ export const generateCSS = (
plainDeclarations.set(key, val, true);
}
});

return (
generateCSSRuleset(
selector, plainDeclarations, stringHandlers, useImportant,
selectorHandlers) +
generatedStyles
const generatedRuleset = generateCSSRuleset(
selector,
plainDeclarations,
stringHandlers,
useImportant,
selectorHandlers,
);


if (generatedRuleset) {
generatedStyles.unshift(generatedRuleset);
}

return generatedStyles;
};

/**
Expand Down
45 changes: 29 additions & 16 deletions src/inject.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ type ProcessedStyleDefinitions = {
// inserted anything yet. We could find this each time using
// `document.querySelector("style[data-aphrodite"])`, but holding onto it is
// faster.
let styleTag = null;
let styleTag /* : ?HTMLStyleElement */ = null;

// Inject a string of styles into a <style> tag in the head of the document. This
// will automatically create a style tag and then continue to use it for
// multiple injections. It will also use a style tag with the `data-aphrodite`
// tag on it if that exists in the DOM. This could be used for e.g. reusing the
// same style tag that server-side rendering inserts.
const injectStyleTag = (cssContents /* : string */) => {
const injectStyleTag = (cssContents /* : string[] */) => {
if (styleTag == null) {
// Try to find a style tag with the `data-aphrodite` attribute first.
styleTag = document.querySelector("style[data-aphrodite]");
styleTag = ((document.querySelector("style[data-aphrodite]") /* : any */) /* : ?HTMLStyleElement */);

// If that doesn't work, generate a new style tag.
if (styleTag == null) {
Expand All @@ -44,12 +44,21 @@ const injectStyleTag = (cssContents /* : string */) => {
}
}

const sheet = ((styleTag.styleSheet || styleTag.sheet /* : any */) /* : CSSStyleSheet */);

if (styleTag.styleSheet) {
// $FlowFixMe: legacy Internet Explorer compatibility
styleTag.styleSheet.cssText += cssContents;
if (sheet.insertRule) {
cssContents.forEach((rule) => {
try {
sheet.insertRule(rule, sheet.cssRules.length);
} catch(e) {
// The selector for this rule wasn't compatible with the browser
}
});
} else {
styleTag.appendChild(document.createTextNode(cssContents));
const currentStyleTag = styleTag;
cssContents.forEach((rule) => {
currentStyleTag.innerText = (currentStyleTag.innerText || '') + rule;
});
}
};

Expand Down Expand Up @@ -113,17 +122,17 @@ const stringHandlers = {
if (val instanceof OrderedElements) {
val.forEach((valVal, valKey) => {
finalVal += generateCSS(
valKey, [valVal], selectorHandlers, stringHandlers, false);
valKey, [valVal], selectorHandlers, stringHandlers, false).join('');
});
} else {
Object.keys(val).forEach(key => {
finalVal += generateCSS(
key, [val[key]], selectorHandlers, stringHandlers, false);
key, [val[key]], selectorHandlers, stringHandlers, false).join('');
});
}
finalVal += '}';

injectGeneratedCSSOnce(name, finalVal);
injectGeneratedCSSOnce(name, [finalVal]);

return name;
} else {
Expand All @@ -137,7 +146,7 @@ const stringHandlers = {
let alreadyInjected = {};

// This is the buffer of styles which have not yet been flushed.
let injectionBuffer = "";
let injectionBuffer = [];

// A flag to tell if we are already buffering styles. This could happen either
// because we scheduled a flush call already, so newly added styles will
Expand All @@ -163,7 +172,7 @@ const injectGeneratedCSSOnce = (key, generatedCSS) => {
asap(flushToStyleTag);
}

injectionBuffer += generatedCSS;
injectionBuffer.push(...generatedCSS);
alreadyInjected[key] = true;
}

Expand All @@ -186,7 +195,7 @@ export const injectStyleOnce = (
};

export const reset = () => {
injectionBuffer = "";
injectionBuffer = [];
alreadyInjected = {};
isBuffering = false;
styleTag = null;
Expand All @@ -200,15 +209,19 @@ export const startBuffering = () => {
isBuffering = true;
};

export const flushToString = () => {
const flushToArray = () => {
isBuffering = false;
const ret = injectionBuffer;
injectionBuffer = "";
injectionBuffer = [];
return ret;
};

export const flushToString = () => {
return flushToArray().join('');
};

export const flushToStyleTag = () => {
const cssContent = flushToString();
const cssContent = flushToArray();
if (cssContent.length > 0) {
injectStyleTag(cssContent);
}
Expand Down
9 changes: 5 additions & 4 deletions tests/generate_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,10 @@ describe('generateCSS', () => {
stringHandlers = {}, useImportant = true) => {
const actual = generateCSS(className, styleTypes, selectorHandlers,
stringHandlers, useImportant);
const expectedNormalized = expected.split('\n').map(x => x.trim()).join('');
const formatStyles = (styles) => styles.replace(/(;|{|})/g, '$1\n');
assert.equal(
const expectedArray = [].concat(expected);
const expectedNormalized = expectedArray.map(rule => rule.split('\n').map(x => x.trim()).join(''));
const formatStyles = (styles) => styles.map(style => style.replace(/(;|{|})/g, '$1\n')).join('');
assert.deepEqual(
actual,
expectedNormalized,
`
Expand Down Expand Up @@ -345,7 +346,7 @@ ${formatStyles(actual)}
color: 'red',
},
color: 'blue',
}], '.foo{color:blue;}.bar .foo{color:red;}', [handler], {}, false);
}], ['.foo{color:blue;}','.bar .foo{color:red;}'], [handler], {}, false);
});

it('correctly prefixes border-color transition properties', () => {
Expand Down
34 changes: 18 additions & 16 deletions tests/index_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
css
} from '../src/index.js';
import { reset } from '../src/inject.js';
import { getSheetText } from './testUtils.js';

describe('css', () => {
beforeEach(() => {
Expand Down Expand Up @@ -80,9 +81,10 @@ describe('css', () => {
asap(() => {
const styleTags = global.document.getElementsByTagName("style");
const lastTag = styleTags[styleTags.length - 1];
const style = getSheetText(lastTag.sheet);

assert.include(lastTag.textContent, `${sheet.red._name}{`);
assert.match(lastTag.textContent, /color:red !important/);
assert.include(style, `${sheet.red._name} {`);
assert.match(style, /color: red !important/);
done();
});
});
Expand Down Expand Up @@ -132,10 +134,10 @@ describe('css', () => {
asap(() => {
const styleTags = global.document.getElementsByTagName("style");
assert.equal(styleTags.length, 1);
const styles = styleTags[0].textContent;
const styles = getSheetText(styleTags[0].sheet);

assert.include(styles, `${sheet.red._name}{`);
assert.include(styles, 'color:red');
assert.include(styles, `${sheet.red._name} {`);
assert.include(styles, 'color: red');

done();
});
Expand Down Expand Up @@ -292,14 +294,14 @@ describe('rehydrate', () => {
asap(() => {
const styleTags = global.document.getElementsByTagName("style");
assert.equal(styleTags.length, 1);
const styles = styleTags[0].textContent;
const styles = getSheetText(styleTags[0].sheet);

assert.notInclude(styles, `.${sheet.red._name}{`);
assert.notInclude(styles, `.${sheet.blue._name}{`);
assert.include(styles, `.${sheet.green._name}{`);
assert.notMatch(styles, /color:blue/);
assert.notMatch(styles, /color:red/);
assert.match(styles, /color:green/);
assert.notInclude(styles, `.${sheet.red._name} {`);
assert.notInclude(styles, `.${sheet.blue._name} {`);
assert.include(styles, `.${sheet.green._name} {`);
assert.notMatch(styles, /color: blue/);
assert.notMatch(styles, /color: red/);
assert.match(styles, /color: green/);

done();
});
Expand Down Expand Up @@ -364,14 +366,14 @@ describe('StyleSheet.extend', () => {
asap(() => {
const styleTags = global.document.getElementsByTagName("style");
assert.equal(styleTags.length, 1);
const styles = styleTags[0].textContent;
const styles = getSheetText(styleTags[0].sheet);

assert.notInclude(styles, '^bar');
assert.include(styles, '.bar .foo');
assert.include(styles, '.baz .bar .foo');
assert.include(styles, 'color:red');
assert.include(styles, 'color:blue');
assert.include(styles, 'color:orange');
assert.include(styles, 'color: red');
assert.include(styles, 'color: blue');
assert.include(styles, 'color: orange');

done();
});
Expand Down
Loading

0 comments on commit 55ed90a

Please sign in to comment.