diff --git a/benchmark/babel.js b/benchmark/babel.js index b5d1694a..ad875792 100644 --- a/benchmark/babel.js +++ b/benchmark/babel.js @@ -1,20 +1,30 @@ import {readFileSync} from 'fs' import {resolve} from 'path' -import Benchmark from 'benchmark' +import {Suite} from 'benchmark' import {transform as babel} from 'babel-core' import plugin from '../src/babel' -const read = path => readFileSync(resolve(__dirname, path), 'utf8') -const fixture = read('./fixtures/babel.js') +const makeTransform = fixturePath => { + const fixture = readFileSync( + resolve(__dirname, fixturePath), + 'utf8' + ) -module.exports = new Benchmark({ - name: 'Babel transform', - minSamples: 500, - fn: () => { - babel(fixture, { - babelrc: false, - plugins: [plugin] - }) - } -}) + return () => babel(fixture, { + babelrc: false, + plugins: [plugin] + }) +} + +const benchs = { + basic: makeTransform('./fixtures/basic.js'), + withExpressions: makeTransform('./fixtures/with-expressions.js') +} + +const suite = new Suite('styled-jsx Babel transform') + +module.exports = + suite + .add('basic', benchs.basic) + .add('with expressions', benchs.withExpressions) diff --git a/benchmark/fixtures/babel.js b/benchmark/fixtures/basic.js similarity index 100% rename from benchmark/fixtures/babel.js rename to benchmark/fixtures/basic.js diff --git a/benchmark/fixtures/with-expressions.js b/benchmark/fixtures/with-expressions.js new file mode 100644 index 00000000..c686bd87 --- /dev/null +++ b/benchmark/fixtures/with-expressions.js @@ -0,0 +1,41 @@ +const c = 'red' +const color = i => i + +export const Test1 = () => ( +
+ test + test +

+

+

+

+

+

+ + +
+) + +export const Test2 = () => test + +export default class { + render() { + return ( +
+

test

+ + + +
+ ) + } +} diff --git a/package.json b/package.json index c1ab5fdf..f389d42e 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,8 @@ ], "dependencies": { "babel-plugin-syntax-jsx": "^6.18.0", + "babel-traverse": "^6.21.0", + "babylon": "^6.14.1", "convert-source-map": "^1.3.0", "object.entries": "^1.0.4", "source-map": "^0.5.6", diff --git a/src/babel.js b/src/babel.js index 78d0c098..99bf7098 100644 --- a/src/babel.js +++ b/src/babel.js @@ -3,6 +3,8 @@ import jsx from 'babel-plugin-syntax-jsx' import hash from 'string-hash' import {SourceMapGenerator} from 'source-map' import convert from 'convert-source-map' +import traverse from 'babel-traverse' +import {parse} from 'babylon' // Ours import transform from '../lib/style-transform' @@ -31,23 +33,117 @@ export default function ({types: t}) { if (isStyledJsx(path)) { const {node} = path return isGlobalEl(node.openingElement) ? - [node] : [] + [path] : [] } - return path.get('children') - .filter(isStyledJsx) - .map(({node}) => node) + return path.get('children').filter(isStyledJsx) } - const getExpressionText = expr => ( - t.isTemplateLiteral(expr) ? - expr.quasis[0].value.raw : - // assume string literal - expr.value - ) + // We only allow constants to be used in template literals. + // The following visitor ensures that MemberExpressions and Identifiers + // are not in the scope of the current Method (render) or function (Component). + const validateExpressionVisitor = { + MemberExpression(path) { + const {node} = path + if ( + t.isThisExpression(node.object) && + t.isIdentifier(node.property) && + ( + node.property.name === 'props' || + node.property.name === 'state' + ) + ) { + throw path.buildCodeFrameError( + `Expected a constant ` + + `as part of the template literal expression ` + + `(eg: ), ` + + `but got a MemberExpression: this.${node.property.name}`) + } + }, + Identifier(path, scope) { + const {name} = path.node + if (scope.hasOwnBinding(name)) { + throw path.buildCodeFrameError( + `Expected \`${name}\` ` + + `to not come from the closest scope.\n` + + `Styled JSX encourages the use of constants ` + + `instead of \`props\` or dynamic values ` + + `which are better set via inline styles or \`className\` toggling. ` + + `See https://github.com/zeit/styled-jsx#dynamic-styles`) + } + } + } + + const getExpressionText = expr => { + const node = expr.node - const makeStyledJsxTag = (id, transformedCss) => ( - t.JSXElement( + // assume string literal + if (t.isStringLiteral(node)) { + return node.value + } + + const expressions = expr.get('expressions') + + // simple template literal without expressions + if (expressions.length === 0) { + return node.quasis[0].value.cooked + } + + // Special treatment for template literals that contain expressions: + // + // Expressions are replaced with a placeholder + // so that the CSS compiler can parse and + // transform the css source string + // without having to know about js literal expressions. + // Later expressions are restored + // by doing a replacement on the transformed css string. + // + // e.g. + // p { color: ${myConstant}; } + // becomes + // p { color: ___styledjsxexpression0___; } + + const replacements = expressions.map((e, id) => ({ + replacement: `___styledjsxexpression_${id}___`, + initial: `$\{${e.getSource()}}` + })).sort((a, b) => a.initial.length < b.initial.length) + + const source = expr.getSource().slice(1, -1) + + const modified = replacements.reduce((source, currentReplacement) => { + source = source.replace( + currentReplacement.initial, + currentReplacement.replacement + ) + return source + }, source) + + return { + source, + modified, + replacements + } + } + + const makeStyledJsxTag = (id, transformedCss, isTemplateLiteral) => { + let css + if (isTemplateLiteral) { + // build the expression from transformedCss + traverse( + parse(`\`${transformedCss}\``), + { + TemplateLiteral(path) { + if (!css) { + css = path.node + } + } + } + ) + } else { + css = t.stringLiteral(transformedCss) + } + + return t.JSXElement( t.JSXOpeningElement( t.JSXIdentifier(STYLE_COMPONENT), [ @@ -57,7 +153,7 @@ export default function ({types: t}) { ), t.JSXAttribute( t.JSXIdentifier(STYLE_COMPONENT_CSS), - t.JSXExpressionContainer(t.stringLiteral(transformedCss)) + t.JSXExpressionContainer(css) ) ], true @@ -65,7 +161,7 @@ export default function ({types: t}) { null, [] ) - ) + } return { inherits: jsx, @@ -124,12 +220,18 @@ export default function ({types: t}) { state.styles = [] + const scope = (path.findParent(path => ( + path.isFunctionDeclaration() || + path.isArrowFunctionExpression() || + path.isClassMethod() + )) || path).scope + for (const style of styles) { // compute children excluding whitespace - const children = style.children.filter(c => ( - t.isJSXExpressionContainer(c) || + const children = style.get('children').filter(c => ( + t.isJSXExpressionContainer(c.node) || // ignore whitespace around the expression container - (t.isJSXText(c) && c.value.trim() !== '') + (t.isJSXText(c.node) && c.node.value.trim() !== '') )) if (children.length !== 1) { @@ -146,23 +248,27 @@ export default function ({types: t}) { `(eg: ), got ${child.type}`) } - const expression = child.expression + const expression = child.get('expression') - if (!t.isTemplateLiteral(child.expression) && - !t.isStringLiteral(child.expression)) { + if (!t.isTemplateLiteral(expression) && + !t.isStringLiteral(expression)) { throw path.buildCodeFrameError(`Expected a template ` + `literal or String literal as the child of the ` + `JSX Style tag (eg: ),` + ` but got ${expression.type}`) } + // Validate MemberExpressions and Identifiers + // to ensure that are constants not defined in the closest scope + child.get('expression').traverse(validateExpressionVisitor, scope) + const styleText = getExpressionText(expression) - const styleId = hash(styleText) + const styleId = hash(styleText.source || styleText) state.styles.push([ styleId, styleText, - expression.loc + expression.node.loc ]) } @@ -187,7 +293,7 @@ export default function ({types: t}) { const [id, css, loc] = state.styles.shift() if (isGlobal) { - path.replaceWith(makeStyledJsxTag(id, css)) + path.replaceWith(makeStyledJsxTag(id, css.source || css, css.modified)) return } @@ -202,17 +308,41 @@ export default function ({types: t}) { }) generator.setSourceContent(filename, state.file.code) transformedCss = [ - transform(String(state.jsxId), css, generator, loc.start, filename), + transform( + String(state.jsxId), + css.modified || css, + generator, + loc.start, + filename + ), convert .fromObject(generator) .toComment({multiline: true}), `/*@ sourceURL=${filename} */` ].join('\n') } else { - transformedCss = transform(String(state.jsxId), css) + transformedCss = transform( + String(state.jsxId), + css.modified || css + ) } - path.replaceWith(makeStyledJsxTag(id, transformedCss)) + if (css.modified) { + transformedCss = css.replacements.reduce( + (transformedCss, currentReplacement) => { + transformedCss = transformedCss.replace( + currentReplacement.replacement, + currentReplacement.initial + ) + return transformedCss + }, + transformedCss + ) + } + + path.replaceWith( + makeStyledJsxTag(id, transformedCss, css.modified) + ) } }, Program: { diff --git a/test/fixtures/expressions.js b/test/fixtures/expressions.js new file mode 100644 index 00000000..3743fb00 --- /dev/null +++ b/test/fixtures/expressions.js @@ -0,0 +1,22 @@ +const color = 'red' +const otherColor = 'green' +const mediumScreen = '680px' + +export default () => ( +
+

test

+ + + + + + + +
+) diff --git a/test/fixtures/expressions.out.js b/test/fixtures/expressions.out.js new file mode 100644 index 00000000..cc317c4d --- /dev/null +++ b/test/fixtures/expressions.out.js @@ -0,0 +1,15 @@ +import _JSXStyle from 'styled-jsx/style'; +const color = 'red'; +const otherColor = 'green'; +const mediumScreen = '680px'; + +export default (() =>
+

test

+ <_JSXStyle styleId={414042974} css={`p.${ color }[data-jsx="2520901095"] {color: ${ otherColor } }`} /> + <_JSXStyle styleId={188072295} css={"p[data-jsx=\"2520901095\"] {color: red }"} /> + <_JSXStyle styleId={806016056} css={`body { background: ${ color } }`} /> + <_JSXStyle styleId={924167211} css={`p[data-jsx="2520901095"] {color: ${ color } }`} /> + <_JSXStyle styleId={3469794077} css={`p[data-jsx="2520901095"] {color: ${ darken(color) } }`} /> + <_JSXStyle styleId={945380644} css={`p[data-jsx="2520901095"] {color: ${ darken(color) + 2 } }`} /> + <_JSXStyle styleId={4106311606} css={`@media (min-width: ${ mediumScreen }) {p[data-jsx="2520901095"] {color: green }p[data-jsx="2520901095"] {color ${ `red` }}}p[data-jsx="2520901095"] {color: red }`} /> +
); diff --git a/test/fixtures/invalid-expressions/1.js b/test/fixtures/invalid-expressions/1.js new file mode 100644 index 00000000..080f525d --- /dev/null +++ b/test/fixtures/invalid-expressions/1.js @@ -0,0 +1,8 @@ +export const Test = (p) => { + return ( +
+

test

+ +
+ ) +} diff --git a/test/fixtures/invalid-expressions/2.js b/test/fixtures/invalid-expressions/2.js new file mode 100644 index 00000000..575240ee --- /dev/null +++ b/test/fixtures/invalid-expressions/2.js @@ -0,0 +1,9 @@ +export function Test(props) { + const {darken} = props + return ( +
+

test

+ +
+ ) +} diff --git a/test/fixtures/invalid-expressions/3.js b/test/fixtures/invalid-expressions/3.js new file mode 100644 index 00000000..154f18ff --- /dev/null +++ b/test/fixtures/invalid-expressions/3.js @@ -0,0 +1,8 @@ +export function Test({color}) { + return ( +
+

test

+ +
+ ) +} diff --git a/test/fixtures/invalid-expressions/4.js b/test/fixtures/invalid-expressions/4.js new file mode 100644 index 00000000..b8244790 --- /dev/null +++ b/test/fixtures/invalid-expressions/4.js @@ -0,0 +1,20 @@ +export class Test { + test() { + const aaaa = 'red' + return ( +
+

test

+ +
+ ) + } + + render() { + return ( +
+

test

+ +
+ ) + } +} diff --git a/test/index.js b/test/index.js index 3fb9f51d..6f226cc4 100644 --- a/test/index.js +++ b/test/index.js @@ -82,8 +82,25 @@ test('should not add the data-jsx attribute to components instances', async t => t.is(code, out.trim()) }) +test('works with expressions in template literals', async t => { + const {code} = await transform('./fixtures/expressions.js') + const out = await read('./fixtures/expressions.out.js') + t.is(code, out.trim()) +}) + +test('throws when using `props` or constants ' + + 'defined in the closest scope', async t => { + [1, 2, 3, 4].forEach(i => { + t.throws( + transform(`./fixtures/invalid-expressions/${i}.js`), + SyntaxError + ) + }) +}) + test('server rendering', t => { function App() { + const color = 'green' return React.createElement('div', null, React.createElement(JSXStyle, { css: 'p { color: red }', @@ -92,13 +109,18 @@ test('server rendering', t => { React.createElement(JSXStyle, { css: 'div { color: blue }', styleId: 2 + }), + React.createElement(JSXStyle, { + css: `div { color: ${color} }`, + styleId: 3 }) ) } // expected CSS const expected = '' + - '' + '' + + '' // render using react ReactDOM.renderToString(React.createElement(App))