From 62c70cce9fe7234130b223ccbe61e47652fbec49 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 7 Jul 2017 18:06:42 -0400 Subject: [PATCH] move css logic into new Stylesheet class, add CSS sourcemap support --- package.json | 4 +- src/generators/Generator.ts | 72 +----- src/generators/Selector.ts | 2 +- src/generators/Stylesheet.ts | 224 ++++++++++++++++++ src/generators/dom/index.ts | 2 +- src/generators/dom/preprocess.ts | 2 +- src/generators/server-side-rendering/index.ts | 2 +- .../server-side-rendering/preprocess.ts | 2 +- src/interfaces.ts | 3 + test/css/index.js | 3 +- test/sourcemaps/index.js | 32 ++- test/sourcemaps/samples/css/input.html | 7 + test/sourcemaps/samples/css/output.css | 6 + test/sourcemaps/samples/css/output.css.map | 12 + test/sourcemaps/samples/css/test.js | 17 ++ yarn.lock | 10 +- 16 files changed, 320 insertions(+), 80 deletions(-) create mode 100644 src/generators/Stylesheet.ts create mode 100644 test/sourcemaps/samples/css/input.html create mode 100644 test/sourcemaps/samples/css/output.css create mode 100644 test/sourcemaps/samples/css/output.css.map create mode 100644 test/sourcemaps/samples/css/test.js diff --git a/package.json b/package.json index 84136453e7..2a0308d430 100644 --- a/package.json +++ b/package.json @@ -60,12 +60,12 @@ "eslint": "^3.12.2", "eslint-plugin-html": "^3.0.0", "eslint-plugin-import": "^2.2.0", - "estree-walker": "^0.3.0", + "estree-walker": "^0.5.0", "fuzzyset.js": "0.0.1", "glob": "^7.1.1", "jsdom": "^9.9.1", "locate-character": "^2.0.0", - "magic-string": "^0.21.1", + "magic-string": "^0.22.1", "mocha": "^3.2.0", "node-resolve": "^1.3.3", "nyc": "^10.0.0", diff --git a/src/generators/Generator.ts b/src/generators/Generator.ts index d1cee03e1a..a2624bc64c 100644 --- a/src/generators/Generator.ts +++ b/src/generators/Generator.ts @@ -17,6 +17,7 @@ import DomBlock from './dom/Block'; import SsrBlock from './server-side-rendering/Block'; import { walkRules } from '../utils/css'; import Selector from './Selector'; +import Stylesheet from './Stylesheet'; import { Node, Parsed, CompileOptions } from '../interfaces'; const test = typeof global !== 'undefined' && global.__svelte_test; @@ -39,12 +40,9 @@ export default class Generator { bindingGroups: string[]; indirectDependencies: Map>; expectedProperties: Set; - cascade: boolean; - css: string; - cssId: string; usesRefs: boolean; - selectors: Selector[]; + stylesheet: Stylesheet; importedNames: Set; aliases: Map; @@ -80,21 +78,11 @@ export default class Generator { this.usesRefs = false; // styles - this.cascade = options.cascade !== false; // TODO remove this option in v2 - this.cssId = parsed.css ? `svelte-${parsed.hash}` : ''; - this.selectors = []; - - if (parsed.css) { - walkRules(parsed.css.children, node => { - node.selector.children.forEach((child: Node) => { - this.selectors.push(new Selector(child)); - }); - }); + this.stylesheet = new Stylesheet(source, parsed, options.filename, options.cascade !== false); - this.css = processCss(this, this.code, this.cascade); - } else { - this.css = null; - } + // TODO this is legacy — just to get the tests to pass during the transition + this.css = this.stylesheet.render(options.cssOutputFilename).css; + this.cssId = `svelte-${parsed.hash}`; // allow compiler to deconflict user's `import { get } from 'whatever'` and // Svelte's builtin `import { get, ... } from 'svelte/shared.ts'`; @@ -231,20 +219,6 @@ export default class Generator { }; } - applyCss(node: Node, stack: Node[]) { - if (!this.cssId) return; - - if (this.cascade) { - if (stack.length === 0) node._needsCssAttribute = true; - return; - } - - for (let i = 0; i < this.selectors.length; i += 1) { - const selector = this.selectors[i]; - selector.apply(node, stack); - } - } - findDependencies( contextDependencies: Map, indexes: Map, @@ -292,7 +266,7 @@ export default class Generator { return (expression._dependencies = dependencies); } - generate(result, options, { name, format }) { + generate(result, options: CompileOptions, { name, format }) { if (this.imports.length) { const statements: string[] = []; @@ -382,6 +356,8 @@ export default class Generator { addString(finalChunk); addString('\n\n' + getOutro(format, name, options, this.imports)); + const { css, cssMap } = this.stylesheet.render(options.cssOutputFilename); + return { ast: this.ast, code: compiled.toString(), @@ -389,7 +365,8 @@ export default class Generator { includeContent: true, file: options.outputFilename, }), - css: this.css, + css, + cssMap }; } @@ -624,31 +601,4 @@ export default class Generator { this.namespace = namespace; this.templateProperties = templateProperties; } - - warnOnUnusedSelectors() { - if (this.cascade) return; - - let locator; - - this.selectors.forEach((selector: Selector) => { - if (!selector.used) { - const pos = selector.node.start; - - if (!locator) locator = getLocator(this.source); - const { line, column } = locator(pos); - - const frame = getCodeFrame(this.source, line, column); - const message = `Unused CSS selector`; - - this.options.onwarn({ - message, - frame, - loc: { line: line + 1, column }, - pos, - filename: this.options.filename, - toString: () => `${message} (${line + 1}:${column})\n${frame}`, - }); - } - }); - } } diff --git a/src/generators/Selector.ts b/src/generators/Selector.ts index e7a23489c1..674cd37702 100644 --- a/src/generators/Selector.ts +++ b/src/generators/Selector.ts @@ -11,7 +11,7 @@ export default class Selector { constructor(node: Node) { this.node = node; - this.blocks = groupSelectors(this.node); + this.blocks = groupSelectors(node); // take trailing :global(...) selectors out of consideration let i = node.children.length; diff --git a/src/generators/Stylesheet.ts b/src/generators/Stylesheet.ts new file mode 100644 index 0000000000..69159e1509 --- /dev/null +++ b/src/generators/Stylesheet.ts @@ -0,0 +1,224 @@ +import MagicString from 'magic-string'; +import { walk } from 'estree-walker'; +import { getLocator } from 'locate-character'; +import Selector from './Selector'; +import getCodeFrame from '../utils/getCodeFrame'; +import { Node, Parsed } from '../interfaces'; + +class Rule { + selectors: Selector[]; + declarations: Node[]; + + constructor(node: Node) { + this.selectors = node.selector.children.map((node: Node) => new Selector(node)); + this.declarations = node.block.children; + } + + apply(node: Node, stack: Node[]) { + this.selectors.forEach(selector => selector.apply(node, stack)); // TODO move the logic in here? + } + + transform(code: MagicString, id: string, keyframes: Map, cascade: boolean) { + const attr = `[${id}]`; + + if (cascade) { + this.selectors.forEach(selector => { + // TODO disable cascading (without :global(...)) in v2 + const { start, end, children } = selector.node; + + const css = code.original; + const selectorString = css.slice(start, end); + + const firstToken = children[0]; + + let transformed; + + if (firstToken.type === 'TypeSelector') { + const insert = firstToken.end; + const head = firstToken.name === '*' ? '' : css.slice(start, insert); + const tail = css.slice(insert, end); + + transformed = `${head}${attr}${tail}, ${attr} ${selectorString}`; + } else { + transformed = `${attr}${selectorString}, ${attr} ${selectorString}`; + } + + code.overwrite(start, end, transformed); + }); + } else { + this.selectors.forEach(selector => selector.transform(code, attr)); + } + + this.declarations.forEach((declaration: Node) => { + const property = declaration.property.toLowerCase(); + if (property === 'animation' || property === 'animation-name') { + declaration.value.children.forEach((block: Node) => { + if (block.type === 'Identifier') { + const name = block.name; + if (keyframes.has(name)) { + code.overwrite(block.start, block.end, keyframes.get(name)); + } + } + }); + } + }); + } +} + +class Atrule { + node: Node; + + constructor(node: Node) { + this.node = node; + } + + transform(code: MagicString, id: string, keyframes: Map) { + if (this.node.name !== 'keyframes') return; + + this.node.expression.children.forEach((expression: Node) => { + if (expression.type === 'Identifier') { + if (expression.name.startsWith('-global-')) { + code.remove(expression.start, expression.start + 8); + } else { + const newName = `${id}-${expression.name}`; + code.overwrite(expression.start, expression.end, newName); + keyframes.set(expression.name, newName); + } + } + }); + } +} + +const keys = {}; + +export default class Stylesheet { + source: string; + parsed: Parsed; + cascade: boolean; + filename: string; + + hasStyles: boolean; + id: string; + + nodes: (Rule|Atrule)[]; + rules: Rule[]; + atrules: Atrule[]; + + constructor(source: string, parsed: Parsed, filename: string, cascade: boolean) { + this.source = source; + this.parsed = parsed; + this.cascade = cascade; + this.filename = filename; + + this.id = `svelte-${parsed.hash}`; + + this.nodes = []; + this.rules = []; + this.atrules = []; + + if (parsed.css && parsed.css.children.length) { + this.hasStyles = true; + + const stack: Atrule[] = []; + let currentAtrule: Atrule = null; + + walk(this.parsed.css, { + enter: (node: Node) => { + if (node.type === 'Atrule') { + const atrule = currentAtrule = new Atrule(node); + stack.push(atrule); + + this.nodes.push(atrule); + this.atrules.push(atrule); + } + + if (node.type === 'Rule' && (!currentAtrule || /(media|supports|document)/.test(currentAtrule.node.name))) { + const rule = new Rule(node); + this.nodes.push(rule); + this.rules.push(rule); + } + }, + + leave: (node: Node) => { + if (node.type === 'Atrule') { + stack.pop(); + currentAtrule = stack[stack.length - 1]; + } + } + }); + } else { + this.hasStyles = false; + } + } + + apply(node: Node, stack: Node[]) { + if (!this.hasStyles) return; + + if (this.cascade) { + if (stack.length === 0) node._needsCssAttribute = true; + return; + } + + for (let i = 0; i < this.rules.length; i += 1) { + const rule = this.rules[i]; + rule.apply(node, stack); + } + } + + render(cssOutputFilename) { + if (!this.hasStyles) { + return { css: null, cssMap: null }; + } + + const code = new MagicString(this.source); + code.remove(0, this.parsed.css.start + 7); + code.remove(this.parsed.css.end - 8, this.source.length); + + const keyframes = new Map(); + this.atrules.forEach((atrule: Atrule) => { + atrule.transform(code, this.id, keyframes); + }); + + this.rules.forEach((rule: Rule) => { + rule.transform(code, this.id, keyframes, this.cascade); + }); + + return { + css: code.toString(), + cssMap: code.generateMap({ + includeContent: true, + source: this.filename, + file: cssOutputFilename + }) + }; + } + + warnOnUnusedSelectors(onwarn) { + if (this.cascade) return; + + let locator; + + this.rules.forEach((rule: Rule) => { + rule.selectors.forEach(selector => { + if (!selector.used) { + const pos = selector.node.start; + + if (!locator) locator = getLocator(this.source); + const { line, column } = locator(pos); + + const frame = getCodeFrame(this.source, line, column); + const message = `Unused CSS selector`; + + onwarn({ + message, + frame, + loc: { line: line + 1, column }, + pos, + filename: this.filename, + toString: () => `${message} (${line + 1}:${column})\n${frame}`, + }); + } + }); + }); + } +} \ No newline at end of file diff --git a/src/generators/dom/index.ts b/src/generators/dom/index.ts index ee04867668..63fefb6687 100644 --- a/src/generators/dom/index.ts +++ b/src/generators/dom/index.ts @@ -61,7 +61,7 @@ export default function dom( const { block, state } = preprocess(generator, namespace, parsed.html); - generator.warnOnUnusedSelectors(); + generator.stylesheet.warnOnUnusedSelectors(options.onwarn); parsed.html.children.forEach((node: Node) => { visit(generator, block, state, node, []); diff --git a/src/generators/dom/preprocess.ts b/src/generators/dom/preprocess.ts index e4ea6f8f9f..b8a569e778 100644 --- a/src/generators/dom/preprocess.ts +++ b/src/generators/dom/preprocess.ts @@ -330,7 +330,7 @@ const preprocessors = { allUsedContexts: [], }); - generator.applyCss(node, elementStack); + generator.stylesheet.apply(node, elementStack); } if (node.children.length) { diff --git a/src/generators/server-side-rendering/index.ts b/src/generators/server-side-rendering/index.ts index ff18b06878..c755b63adc 100644 --- a/src/generators/server-side-rendering/index.ts +++ b/src/generators/server-side-rendering/index.ts @@ -27,7 +27,7 @@ export class SsrGenerator extends Generator { preprocess(this, parsed.html); - this.warnOnUnusedSelectors(); + this.stylesheet.warnOnUnusedSelectors(options.onwarn); if (templateProperties.oncreate) removeNode( diff --git a/src/generators/server-side-rendering/preprocess.ts b/src/generators/server-side-rendering/preprocess.ts index c82fda9e9b..1f6effc5f7 100644 --- a/src/generators/server-side-rendering/preprocess.ts +++ b/src/generators/server-side-rendering/preprocess.ts @@ -61,7 +61,7 @@ const preprocessors = { generator.components.has(node.name) || node.name === ':Self'; if (!isComponent) { - generator.applyCss(node, elementStack); + generator.stylesheet.apply(node, elementStack); } if (node.children.length) { diff --git a/src/interfaces.ts b/src/interfaces.ts index 78fc8730b9..668c9816f8 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -40,6 +40,9 @@ export interface CompileOptions { filename?: string; generate?: string; + outputFilename?: string; + cssOutputFilename?: string; + dev?: boolean; shared?: boolean | string; cascade?: boolean; diff --git a/test/css/index.js b/test/css/index.js index 6c56238ce2..93f7c218ea 100644 --- a/test/css/index.js +++ b/test/css/index.js @@ -25,12 +25,13 @@ describe("css", () => { // add .solo to a sample directory name to only run that test const solo = /\.solo/.test(dir); + const skip = /\.skip/.test(dir); if (solo && process.env.CI) { throw new Error("Forgot to remove `solo: true` from test"); } - (solo ? it.only : it)(dir, () => { + (solo ? it.only : skip ? it.skip : it)(dir, () => { const config = tryRequire(`./samples/${dir}/_config.js`) || {}; const input = fs .readFileSync(`test/css/samples/${dir}/input.html`, "utf-8") diff --git a/test/sourcemaps/index.js b/test/sourcemaps/index.js index 2209ad0f7d..91612e6aa3 100644 --- a/test/sourcemaps/index.js +++ b/test/sourcemaps/index.js @@ -21,34 +21,50 @@ describe("sourcemaps", () => { `test/sourcemaps/samples/${dir}/input.html` ); const outputFilename = path.resolve( - `test/sourcemaps/samples/${dir}/output.js` + `test/sourcemaps/samples/${dir}/output` ); const input = fs.readFileSync(filename, "utf-8").replace(/\s+$/, ""); - const { code, map } = svelte.compile(input, { + const { code, map, css, cssMap } = svelte.compile(input, { filename, - outputFilename + outputFilename: `${outputFilename}.js`, + cssOutputFilename: `${outputFilename}.css` }); fs.writeFileSync( - outputFilename, + `${outputFilename}.js`, `${code}\n//# sourceMappingURL=output.js.map` ); fs.writeFileSync( - `${outputFilename}.map`, + `${outputFilename}.js.map`, JSON.stringify(map, null, " ") ); + if (css) { + fs.writeFileSync( + `${outputFilename}.css`, + `${css}\n/*# sourceMappingURL=output.css.map */` + ); + fs.writeFileSync( + `${outputFilename}.css.map`, + JSON.stringify(cssMap, null, " ") + ); + } + assert.deepEqual(map.sources, ["input.html"]); + if (cssMap) assert.deepEqual(cssMap.sources, ["input.html"]); const { test } = require(`./samples/${dir}/test.js`); - const smc = new SourceMapConsumer(map); - const locateInSource = getLocator(input); + + const smc = new SourceMapConsumer(map); const locateInGenerated = getLocator(code); - test({ assert, code, map, smc, locateInSource, locateInGenerated }); + const smcCss = cssMap && new SourceMapConsumer(cssMap); + const locateInGeneratedCss = getLocator(css || ''); + + test({ assert, code, map, smc, smcCss, locateInSource, locateInGenerated, locateInGeneratedCss }); }); }); }); diff --git a/test/sourcemaps/samples/css/input.html b/test/sourcemaps/samples/css/input.html new file mode 100644 index 0000000000..ad0845d15b --- /dev/null +++ b/test/sourcemaps/samples/css/input.html @@ -0,0 +1,7 @@ +

red

+ + \ No newline at end of file diff --git a/test/sourcemaps/samples/css/output.css b/test/sourcemaps/samples/css/output.css new file mode 100644 index 0000000000..9f3e676b54 --- /dev/null +++ b/test/sourcemaps/samples/css/output.css @@ -0,0 +1,6 @@ + + [svelte-2772200924].foo, [svelte-2772200924] .foo { + color: red; + } + +/*# sourceMappingURL=output.css.map */ \ No newline at end of file diff --git a/test/sourcemaps/samples/css/output.css.map b/test/sourcemaps/samples/css/output.css.map new file mode 100644 index 0000000000..c77b6d62e7 --- /dev/null +++ b/test/sourcemaps/samples/css/output.css.map @@ -0,0 +1,12 @@ +{ + "version": 3, + "file": "output.css", + "sources": [ + "input.html" + ], + "sourcesContent": [ + "

red

\n\n" + ], + "names": [], + "mappings": "AAEO;CACN,iDAAI;;;AAGL" +} \ No newline at end of file diff --git a/test/sourcemaps/samples/css/test.js b/test/sourcemaps/samples/css/test.js new file mode 100644 index 0000000000..0c9d7e9b6e --- /dev/null +++ b/test/sourcemaps/samples/css/test.js @@ -0,0 +1,17 @@ +export function test ({ assert, smcCss, locateInSource, locateInGeneratedCss }) { + const expected = locateInSource( '.foo' ); + + const loc = locateInGeneratedCss( '.foo' ); + + const actual = smcCss.originalPositionFor({ + line: loc.line + 1, + column: loc.column + }); + + assert.deepEqual( actual, { + source: 'input.html', + name: null, + line: expected.line + 1, + column: expected.column + }); +} diff --git a/yarn.lock b/yarn.lock index a3b9fbe5a4..45c20cdf10 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1224,6 +1224,10 @@ estree-walker@^0.3.0: version "0.3.1" resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-0.3.1.tgz#e6b1a51cf7292524e7237c312e5fe6660c1ce1aa" +estree-walker@^0.5.0: + version "0.5.0" + resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-0.5.0.tgz#aae3b57c42deb8010e349c892462f0e71c5dd1aa" + esutils@^2.0.2: version "2.0.2" resolved "https://registry.yarnpkg.com/esutils/-/esutils-2.0.2.tgz#0abf4f1caa5bcb1f7a9d8acc6dea4faaa04bac9b" @@ -1972,9 +1976,9 @@ magic-string@^0.19.0, magic-string@~0.19.0: dependencies: vlq "^0.2.1" -magic-string@^0.21.1: - version "0.21.3" - resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.21.3.tgz#87e201009ebfde6f46dc5757305a70af71e31624" +magic-string@^0.22.1: + version "0.22.1" + resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.22.1.tgz#a1bda64dfd4ae6c63797a45a67ee473b1f8d0e0f" dependencies: vlq "^0.2.1"