Format CSS in template literals with expressions by pomber · Pull Request #2102 · prettier/prettier

pomber · 2017-06-11T14:49:48Z

This adds support for formatting CSS inside template literals with expressions (#1948). For example:

<style jsx>{`
  div {
    animation: linear ${seconds}s ease-out;
  }
`}</style>;

Currently is only working for styled-jsx, but if you like the approach, I can add styled-components too, it should be easy.

It works like this:

It replace all the expressions from the template literal with @prettier-placeholder
Parse and print the template literal (css) doc
Print the doc for each expression
Traverse the css doc replacing all the placeholders with the expression docs

If it fails to find or replace any of the placeholders it throws an error and abort the formatting of the template literal.

vjeux · 2017-06-11T15:14:45Z

Currently is only working for styled-jsx, but if you like the approach, I can add styled-components too, it should be easy.

FYI, styled-jsx doesn't support dynamic expressions like this, only styled-components do ;)

vjeux · 2017-06-11T15:22:41Z

src/multiparser.js

+function wrapCssTemplate(quasisDoc, expressionDocs) {
+  const allReplaced = replacePlaceholders(quasisDoc, expressionDocs);
+  if (!allReplaced) {
+    throw new Error("Couldn't insert all the expressions");


Could you use createError with the right location, this way if it fires people won't be as confused.

Ok, but this won't be shown to users, it's there just in case a placeholder was parsed in an odd way and we couldn't find it. Throwing the error aborts the formatting of the template literal but the rest of the file should be formatted normally.
Maybe it should be an assert.

You are right, let's just leave it like this. If it becomes an issue, people will report it and we'll figure out a solution.

vjeux · 2017-06-11T15:26:50Z

src/multiparser.js

+    ) {
+      // If placeholder is splitted, join it
+      const [at, placeholder, ...rest] = doc.parts;
+      doc.parts = [at + placeholder, ...rest];


Could you re-create a doc tree while you are iterating instead of doing mutations? It's going to bite us in the future I'm sure :(

I'm having problems trying to find a clean way to re-create the doc. Is the mutation during the iteration what you don't like? If so, I can postpone the mutations until the traverse finishes.

vjeux · 2017-06-11T15:30:08Z

src/multiparser.js

+        const text = rawQuasis.join("@prettier-placeholder");
+
        return {
          parser: "postcss",


We should make all the CSS go through the same codepath instead of duplicating this non trivial logic.

vjeux · 2017-06-11T15:31:56Z

This is so exciting! Thanks for building it <3

If you can build a new doc tree instead of mutating it and merging the CSS codepaths, I'll merge it. I'm so excited :)

pomber · 2017-06-11T15:55:20Z

Thanks, I'll try to fix it later today.

Do you know why Node v4 does not like the PR?

For the styled-jsx test cases, I'm using what @giuseppeg recommended in this issue

vjeux · 2017-06-11T16:03:25Z

Probably crashes on destructuring for node 4. You can use nvm to install node 4 and try it yourself.

Good to know about styled-jsx that supports expressions, I thought it didn't.

giuseppeg · 2017-06-11T16:14:30Z

Good to know about styled-jsx that supports expressions, I thought it didn't.

We do support expressions (constants) but we don't allow the usage of props or anything that is in the component scope. Basically we try to stop people from putting dynamic code inside of expressions because we don't support them yet. You can find the relevant code in the utils file.

@pomber you may want to update the fixture, I added more use cases.

vjeux · 2017-06-11T20:19:44Z

Can you use mapDoc to rebuild it?

azz · 2017-06-11T23:11:29Z

tests/template_literals/__snapshots__/jsfmt.spec.js.snap

+    animation: 3s
+      ease-in
+      1s
+      \${foo => foo.getIterations()}


azz · 2017-06-11T23:12:20Z

This is so exciting!

Is there a chance that someone will type @prettier-placeholder and break this?

pomber · 2017-06-11T23:26:29Z

Mmm, it won't break, but the template won't be formatted. So you could use @prettier-placeholder as a escape hatch :p

giuseppeg · 2017-06-12T07:15:05Z

src/multiparser.js

+          (/^[A-Z]/.test(parent.tag.object.name) &&
+            parent.tag.property.name === "extend"));
+
+      if (isStyledJsx || isStyledComponents) {


nit: isStyledComponents and isStyledJsx could be functions that get the current path as input.

Then you can make an helper function isCss:

export const isCss = path => [isStyledJsx, isStyledCompoents].some(test => test(path))

This way you can add tests for other libraries easily.

azz · 2017-06-12T07:46:03Z

Just FYI there's probably going to be merge conflicts with #2109.

pomber · 2017-06-13T15:01:44Z

Done with the merge, thanks @azz :p

vjeux · 2017-06-13T16:45:19Z

<3 <3 <3 <3

vjeux reviewed Jun 11, 2017

View reviewed changes

pomber force-pushed the css-in-template-literals-with-expressions branch from 3a6fcba to 6050530 Compare June 11, 2017 21:49

azz reviewed Jun 11, 2017

View reviewed changes

tests/template_literals/__snapshots__/jsfmt.spec.js.snap

animation: 3s

ease-in

1s

\${foo => foo.getIterations()}

Copy link
Copy Markdown

Member

azz Jun 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😁

giuseppeg reviewed Jun 12, 2017

View reviewed changes

pomber added 7 commits June 13, 2017 10:54

Add support for styled-jsx with expressions

eb14d20

Lint

2a48d72

Fix require for Node 4

ee4ec57

Re-create template-literal document when replacing placeholders

d320af6

Add support for styled-components with expressions

ccd491f

Fix merge

518ba12

Move css library detection to functions

577d189

pomber force-pushed the css-in-template-literals-with-expressions branch from fd060c7 to 577d189 Compare June 13, 2017 14:36

vjeux merged commit 8dd0cb2 into prettier:master Jun 13, 2017

pomber deleted the css-in-template-literals-with-expressions branch June 13, 2017 17:29

lydell mentioned this pull request Jun 14, 2017

There is an intention to support es6 template strings ？ #2130

Closed

renovate bot mentioned this pull request Jun 27, 2017

Update dependency prettier to version 1.5.2 renovatebot/renovate#365

Merged

lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 20, 2019

lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019

Uh oh!

Conversation

pomber commented Jun 11, 2017

Uh oh!

vjeux commented Jun 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vjeux commented Jun 11, 2017

Uh oh!

pomber commented Jun 11, 2017

Uh oh!

vjeux commented Jun 11, 2017

Uh oh!

giuseppeg commented Jun 11, 2017

Uh oh!

vjeux commented Jun 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azz commented Jun 11, 2017

Uh oh!

pomber commented Jun 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azz commented Jun 12, 2017

Uh oh!

pomber commented Jun 13, 2017

Uh oh!

vjeux commented Jun 13, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants