Linebreaks weirdly in long ES6 template strings with expressions

[Swizec]

Prettier 1.8.2 Playground link

# Options (if any):
--print-width=80

Input:

class Quote {
    character = {
        name: 'Dorian'
    }

    text = `Yes, ${this.character.name}, you will always be fond of me. I represent to you all the sins you have never had the courage to commit.`
}

Output:

class Quote {
  character = {
    name: "Dorian"
  };

  text = `Yes, ${
    this.character.name
  }, you will always be fond of me. I represent to you all the sins you have never had the courage to commit.`;
}

Expected behavior:

I would prefer a long string to not get split into multiple lines, even if it contains expressions. When there are no expressions, Prettier correctly lets a long string go beyond the print-width limit.

I can't decide if this is a bug or a personal preference, but it's been creating weird hard to read code in our codebase. We can clean most of it up by simplifying our expressions, but there's only so short we can go.

[ikatyang]

Ref: previous discussion https://github.com/prettier/prettier/issues/3315#issuecomment-346897564

[duailibe]

As we'll focus the discussion on this issue, I'll copy my comment from the other thread (cc @lipis):

The thing about template literals is without breaking between { and } we were actually printing:

const str1 = `text text text ${foo.
  bar} text text`;

const str2 = `text text text ${foo[
  bar
]} text text`;

which IMO are strictly worse than what we're printing right now. Note the foo.bar and foo[bar] are a group with "break points", meaning that if they don't fit in one line, they will break. We'd need to special case printing MemberExpressions when inside a template literal so this wouldn't happen.

The other problem with this is when should and shouldn't break in those cases:

foo = `Text text text text text ${foo.bar.baz.spam.eggs}`;
foo = `Text text text text text ${longFooBarBazSpam.longFooBarBazEggs}`;

Until we can find an heurisitic that's good enough to decide on those cases, it's better to keep consistent that always breaks.

[lipis]

I think those cases we shouldn't brake at all..

foo = `Text text text text text ${foo.bar.baz.spam.eggs}`;
foo = `Text text text text text ${longFooBarBazSpam.longFooBarBazEggs}`;

and consider that a single expression

while this one is not:

foo = `Text text text text text ${foo.bar.baz.spam.eggs + foo.bar.baz.bacon}`;
foo = `Text text text text text ${longFooBarBazSpam.longFooBarBazEggs - longFooBarBazSpam.longFooBarBazBacon}`;

[Swizec]

Personally I am of the opinion that a template string, regardless of what’s in there, is a single expression. People putting too much logic in there is an antipattern and almost always leads to a mess.

Discouraging messes is the job of a linter, not a formatter, if I’m not mistaken. Maybe a good way to approach this is to have people vote on whether they perceive complex string templates as one expression or more?

On Wed, Dec 6, 2017 at 6:33 AM Lipis [email protected] wrote:

I think those cases we shouldn't brake at all..

foo = Text text text text text ${foo.bar.baz.spam.eggs}; foo = Text text text text text ${longFooBarBazSpam.longFooBarBazEggs};

and consider that a single expression

while this one is not:

foo = Text text text text text ${foo.bar.baz.spam.eggs + foo.bar.baz.bacon}; foo = Text text text text text ${longFooBarBazSpam.longFooBarBazEggs - longFooBarBazSpam.longFooBarBazBacon};

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/prettier/prettier/issues/3368#issuecomment-349613128, or mute the thread https://github.com/notifications/unsubscribe-auth/AADeM-Fq6oU30YxsD1pd5H5hrhiYl-dSks5s9nujgaJpZM4Qx-k7 .

[lipis]

I would agree.. maybe it makes sense to not break them at all..

[duailibe]

Discouraging messes is the job of a linter, not a formatter

The job of a formatter is to format the code, whether or not it's a mess, trying its best to output code that looks consistent. I don't think we should be in charge of encouraging or discouraging any pattern.

Don't get me wrong, I would love if people wrote code in specific styles.. that would make the job of a formatter a whole lot simpler, but that's not what we've noticed out there in codebases, both smaller and bigger ones.

As for the formatting issue, prettier used to not format template literals at all, but people complained code wasn't getting formatted. Then it started formatting but using removeLines to remove breaks which would output ${a.b.c.d.e} in a single line, but would break other more complex cases, with function calls and object expressions inside.

I see what we have today as a good compromise but I agree "simple" cases could get better. The whole point is deciding what is simple and have a sane way to format those cases without adding too much complexity to the code.

See #2046, #1893, #1662, #1626, #821, #1183 (and there's more :-)) for more context on this issue.

[jwbay]

If there's a rule of thumb needed, I'd propose that inserting one existing value into a template string should never cause breaks. If you're doing something that could create a new value (getters aside), or picks between values, it's complex enough to warrant breaks.

Expressions that shouldn't break:

`${foo}` // this is done, which is awesome!
`${foo.bar}`
`${foo[bar]}`
`${foo[bar][baz]}`

There's some interesting middle ground that would be nice to have, but breaks with that rule of thumb:

`${foo()}` // all of the above as function calls with no arguments. i.e. just 'get this computed value'
`${foo || bar}` // to inline defaults without paying the 'break cost'. but with member access chains, this could get long. :shrug:

[danwfeeney]

I'd say a lot of people, including myself, could dig an option that reverts to previous behaviour i.e. rule to disable formatting template literals completely e.g. ignoreTemplateLiterals: true.

Does this exist already? Any reason this would be a problem?

[JonathanWolfe]

I definitely think that template literals should be treated as one holistic expressions and their contents should never be altered.

A travesty of formatting

log(
	chalk.bgRed(
		chalk.white(
			`Covered Lines below threshold: ${
				coverageSettings.lines
			}%. Actual: ${coverageSummary.total.lines.pct}%`
		)
	)
);

(Github displaying tabs as 8 spaces is also a travesty)

[j-f1]

@JonathanWolfe Would you agree that this should not be formatted?

`Covered Lines below threshold: ${
     coverageSettings
.lines

}%. Actual: ${  coverageSummary

.total.

lines
.
pct
  }%`

(that’s valid JS)

[JonathanWolfe]

@j-f1: Unfortunately, and it pains me to say this to that example, yes; I would leave that alone.

The copout answer is because nobody would write something like that, but really, I believe the contents of Template Strings/Literals/Expressions to be intentionally formatted by the author when they're written.

An ideal answer might be to leave the expression location alone, but format them as if it is a standalone line. This way the contents get formatted and made pretty, but the braces aren't broken apart.

Maybe an even simpler answer is to not allow line breaks when formatting template expressions.

[duailibe]

@JonathanWolfe We used to not format and we changed it because a LOT of people expected the code to be formatted there as well. So it's not an option to stop formatting those expressions.

We do want to make the formatting better and we'll gladly take suggestions

[JonathanWolfe]

@duailibe: I get that expectation. I would say the best thing to do would be to not linebreak the braces of ${ } as it just makes the the template literal look awkward afterwards

[duailibe]

Yeah that's an option.. If we simply remove the linebreaks, that would be formatted as:

const foo = `some text here ${foo
  .bar.baz} continues here`;

Which is worse than what we're doing right now. That's just how it prints a MemberExpression. So in that case we'd also need to special case if a MemberExpression is inside a TemplateLiteral, but what would mean for examples like:

const foo = `some text here ${foo.something({
  key: prop
}).bar.baz()}`;

Should it remain as is? That's inconsistent with how we print that case in other places:

foo
  .something({
    key: prop
  })
  .bar.baz();

Anyway, we're not opposed on improving that, it's just it's a difficult problem and we'd need a consistent suggestion of different examples and how we should print them. Hopefully that gives more context on the decisions on this subject.

[JonathanWolfe]

@duailibe: Sure, I've got lots of time right now so I'll make a big list of before/afters.

I went out and took a bunch of template literals from MDN, CSS-Tricks, and the lit-html repo.

It appears to me that the heuristic to use to get the best formatting is:

• Format content inside template expressions as if they were any normal expression
• Never add new lines, except:
  • When a function is present AND a parameter to that function is another function or another template literal

// Input
console.log(`Fifteen is ${a          + b} and not ${2*a    +
      
      b}.`);

// Current
console.log(`Fifteen is ${a + b} and not ${2 * a + b}.`);

// Desired - No change
console.log(`Fifteen is ${a + b} and not ${2 * a + b}.`); 

// Input
const foo = `some text here ${foo.something({
  key: prop
}).bar.baz()}`;

// Current
const foo = `some text here ${foo
  .something({
    key: prop,
  })
  .bar.baz()}`;

// Desired
const foo = `some text here ${foo.something({key: prop}).bar.baz()}`

// Input
const classes = `header ${ isLargeScreen() ? '' :
  (item.isCollapsed ? 'icon-expander' : 'icon-collapser') }`;
const classes = `header ${ isLargeScreen() ? '' :
 `icon-${(item.isCollapsed ? 'expander' : 'collapser')}` }`;

// Current
const classes = `header ${
  isLargeScreen() ? '' : item.isCollapsed ? 'icon-expander' : 'icon-collapser'
}`;
const classes = `header ${
  isLargeScreen() ? '' : `icon-${item.isCollapsed ? 'expander' : 'collapser'}`
}`;

// Desired
const classes = `header ${isLargeScreen() ? '' : item.isCollapsed ? 'icon-expander' : 'icon-collapser'}`;
const classes = `header ${isLargeScreen() ? '' : `icon-${item.isCollapsed ? 'expander' : 'collapser'}`}`;

// Input
var pronoun = person.parent.getPronoun({formal:true});
var output = myTag`that ${person.parent.getPronoun({ formal:true })} is a ${ person.parent.getAge() }`;

// Current
var pronoun = person.parent.getPronoun({ formal: true });
var output = myTag`that ${person.parent.getPronoun({
  formal: true,
})} is a ${person.parent.getAge()}`;

// Desired
var pronoun = person.parent.getPronoun({ formal: true });
var output = myTag`that ${person.parent.getPronoun({ formal: true })} is a ${person.parent.getAge()}`;

// Input
let person = {
  firstName: `Ryan`,
  lastName: `Christiani`,
  sayName() {
    return `Hi my name is ${this.firstName} ${this.lastName}`;
  }
};

// Current
let person = {
  firstName: `Ryan`,
  lastName: `Christiani`,
  sayName() {
    return `Hi my name is ${this.firstName} ${this.lastName}`;
  },
};

// Desired - No change
let person = {
  firstName: `Ryan`,
  lastName: `Christiani`,
  sayName() {
    return `Hi my name is ${this.firstName} ${this.lastName}`;
  },
};

// Input
function createMarkup(data) {
  return `
    <article class="pokemon">
      <h3>${data.name}</h3>
      <p>The Pokemon ${data.name} has a base experience of ${data.base_experience}, they also weigh ${data.weight}</p>
    </article>
  `
}

// Current
function createMarkup(data) {
  return `
    <article class="pokemon">
      <h3>${data.name}</h3>
      <p>The Pokemon ${data.name} has a base experience of ${
    data.base_experience
  }, they also weigh ${data.weight}</p>
    </article>
  `;
}

// Desired
function createMarkup(data) {
  return `
    <article class="pokemon">
      <h3>${data.name}</h3>
      <p>The Pokemon ${data.name} has a base experience of ${data.base_experience}, they also weigh ${data.weight}</p>
    </article>
  `;
}

// Input
class element {
  render() {
    return svg`<g>
      ${[0, 10, 20].map((x, i) => svg`<line x1=${x * i} y1="0" x2=${x * i} y2="20"/>`)}
      ${[0, 10, 20].map((y, i) => svg`<line x1="0" y1=${y + i} x2="0" y2=${y + i}/>`)}
    </g>`;
  }
}

// Current
class element {
  render() {
    return svg`<g>
      ${[0, 10, 20].map(
        (x, i) => svg`<line x1=${x * i} y1="0" x2=${x * i} y2="20"/>`
      )}
      ${[0, 10, 20].map(
        (y, i) => svg`<line x1="0" y1=${y + i} x2="0" y2=${y + i}/>`
      )}
    </g>`;
  }
}

// Desired - no change
class element {
  render() {
    return svg`<g>
      ${[0, 10, 20].map(
        (x, i) => svg`<line x1=${x * i} y1="0" x2=${x * i} y2="20"/>`
      )}
      ${[0, 10, 20].map(
        (y, i) => svg`<line x1="0" y1=${y + i} x2="0" y2=${y + i}/>`
      )}
    </g>`;
  }
}

// Input
const render = () => html`
  <ul>
  ${repeat(items, (i) => i.id, (i, index) => html`
    <li>${index}: ${i.name}</li>`)}
  </ul>
`;

// Current
const render = () => html`
  <ul>
  ${repeat(
    items,
    i => i.id,
    (i, index) => html`
    <li>${index}: ${i.name}</li>`
  )}
  </ul>
`;

// Desired - no change
const render = () => html`
  <ul>
  ${repeat(
    items,
    i => i.id,
    (i, index) => html`
    <li>${index}: ${i.name}</li>`
  )}
  </ul>
`;

// Input
const render = () => html`
  ${when(state === 'loading',
  html`<div>Loading...</div>`,
  html`<p>${message}</p>`)}
`;

// Current
const render = () => html`
  ${when(
    state === 'loading',
    html`<div>Loading...</div>`,
    html`<p>${message}</p>`
  )}
`;

// Desired - No change
const render = () => html`
  ${when(
    state === 'loading',
    html`<div>Loading...</div>`,
    html`<p>${message}</p>`
  )}
`;

// Input
const render = () => html`
  <div>Current User: ${guard(user, () => user.getProfile())}</div>
`;

// Current
const render = () => html`
  <div>Current User: ${guard(user, () => user.getProfile())}</div>
`;

// Desired
const render = () => html`
  <div>Current User: ${guard(
    user, 
    () => user.getProfile()
  )}</div>
`;

// Input
const omg = `Covered Lines below threshold: ${
   coverageSettings
.lines

}%. Actual: ${  coverageSummary

.total.

lines
.
pct
  }%`

// Current
const omg = `Covered Lines below threshold: ${
  coverageSettings.lines
}%. Actual: ${coverageSummary.total.lines.pct}%`;

// Desired
const omg = `Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%`;

// Input
log(
  chalk.bgRed(
    chalk.white(
      `Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%`
    )
  )
);

// Current
log(
  chalk.bgRed(
    chalk.white(
      `Covered Lines below threshold: ${
        coverageSettings.lines
      }%. Actual: ${coverageSummary.total.lines.pct}%`
    )
  )
);

// Desired
log(
  chalk.bgRed(
    chalk.white(
      `Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%`
    )
  )
);

// Input
render(html`
  Count: <span>${asyncReplace(countUp())}</span>.
`, document.body);

// Current
render(
  html`
  Count: <span>${asyncReplace(countUp())}</span>.
`,
  document.body
);

// Desired - No change
render(
  html`
  Count: <span>${asyncReplace(countUp())}</span>.
`,
  document.body
);

Playground Link with examples and current output

[lachieh]

It's been a month since @JonathanWolfe's suggestion and a few thumbsups on the post. How do we keep it moving? Someone want to do a PR?

[framerate]

I just started using prettier and to immediately turn it off because of this! Definitely willing to help out if necessary!

[lachieh]

*tumbleweeds and crickets*

Would a PR for this be welcomed?

[lydell]

A PR exploring ways to improve the oh-so-tricky-to-format template literals would certainly be up for discussion :+1:

[caub]

I think with template literal, it shouldn't try to respect printWidth at all (leaving it manual), or with a configuration option for this as template literal are more and more important

Other example https://prettier.io/playground/#N4Igxg9gdgLgprEAuc0DOMAEAzCFMC8mABgBZoA2AhgBQAkwAslTKQHQBOEArlACY1SASgC+AGgbNWnHvxppMAKkwBGAAxrRAUglMW7LrwEUlqjdt0QADlTABLGAE8RQ4gG4QYkNZh30yUCoOLgB3AAUghDRkECoANwg7Pk9YjGRsKgo0OC8AIw5bAGs4GABlG3soAHNkGA5uHJA+CDB0zOyvOyhsjhgwgqqAWyo2rMaAKzQADwAhArBisqpBuAAZLrhRju9uGCtdgCYtxpsOHpjcqlzHCmgUqw4umAB1JNZkAA41LweIbOeClYYg84D04psvBw4ABHbh2KH9KhDEZIDJjLzZQZ2Wr1RpoLpVChwACK3Ag8GOXhgV1efHeSAOVIKdgoBIAwhBBsMYlBoBCQNxsgAVK7RVHtOAiERAA

[tgroutars]

Templates strings are still string, and it's helpful to display them as is, hence never ever add a new line. If you need to call a function in a template string, then just declare a variable. At least having the option to make it behave that way would be great

[suchipi]

@tgroutars we will keep simple variables as one-line, but more "complex" things get broken.

Here is the current behavior (all lines exceed the print width):

Prettier 1.11.1 Playground link

--range-end 9999999
--range-start 0

Input:

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${name}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${attributes.name}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${model.attributes.name}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${model.getName()}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${model.get("name")}`;

Output:

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${name}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${
  attributes.name
}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${
  model.attributes.name
}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${model.getName()}`;

`This is really long bla bla bla bla bla bla bla bla bla bla bla bla bla bla ${model.get(
  "name"
)}`;

[tgroutars]

@suchipi That's awesome! :)

[caub]

@suchipi do you think a configuration like I proposed is possible? because for those occasional large template strings, I really prefer letting my code editor wrap it if needed

[lachieh]

@suchipi That's the same behaviour as when this was posted back in v1.8.2. I'm the same as OP—I pretty much never want template literals to wrap.

I understand the opinionatedness of Prettier, but if the idea is to remove the need to think about the code formatting, then wrapping template literals goes against that. I have to think much harder about this:

const example = `${person.fullName}'s parent is called ${
  person.parent.fullName
} and is of age ${person.parent.age}`;

than I do about this:

const example = `${person.fullName}'s parent is called ${person.parent.fullName} and is of age ${person.parent.age}`;

To get around this, I would have to define three extra variables, even if they only get used this one time. That's not something that will get optimized away in a minifier/webpack.

const personName = person.fullName;
const parentName = person.parent.fullName;
const parentAge = person.parent.age;
const example = `${personName}'s parent is called ${parentName} and is of age ${parentAge}`;

What determines whether there should be an option for preferences like this?

[suchipi]

@caub I don't see your proposal, could you post a link to it?

@lachieh we aren't satisfied with this output either, but template literal strings have been a somewhat difficult thing to get right. At one point in the past we removed all newlines and it broke some things (like styled-components). I'd be open to changing the current formatting rules to be more inclusive for which things should not break; I don't think there's a need for an option here (and I would prefer not having one).

[caub]

Sorry, I just meant my previous comment, about having something like a ignoreTemplateLiterals config option to avoid formatting them, even if they exceed printWidth

[suchipi]

We won't do that; please read https://prettier.io/docs/en/option-philosophy.html for more info.

[caub]

Ok thanks I know the options must remain very simple. So then I'd suggest not trying to line-break template literals on their expressions by default. And thanks again for this tool

[zcmgyu]

I know my answer isn't your problem but I just want to share for somebody try to find same problem with ESLint. In my .eslintrc file I added indent: ['error', 'tab'],, it make my template literal break new line. Just remove it