Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make "jsx-bracket-same-line" More Generalised? #5593

Closed
marbuser opened this issue Dec 4, 2018 · 6 comments
Closed

Make "jsx-bracket-same-line" More Generalised? #5593

marbuser opened this issue Dec 4, 2018 · 6 comments
Labels
lang:html Issues affecting HTML (and SVG but not JSX) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:duplicate Issues that are a duplicate of a previous issue

Comments

@marbuser
Copy link

marbuser commented Dec 4, 2018

Tried to report the issue using the playground but GitHub wasn't having it so going to write this manually. Apologies if the format isn't correct. I will try to make it as neat as possible.

Currently the option 'jsx-bracket-same-line' is obviously only working with JSX. With the recent implementation of HTML, Vue, and Angular support, I think this rule should be reworked (or a variant/option added to it) that generalises it's use-case.

For example, I'm a VueJS user and when using Prettier to format my HTML, I don't get an option to decide whether I want the < > brackets on the same line or not. I get that the whole point of Prettier is to be 'opinionated', but if people who use JSX (AKA React) already have this option, then I think adding some option/rule to accomodate the new formatting support for HTML, Vue, and Angular won't make Prettier any more/less 'opinionated' than it currently is.

Example:
Default VueJS CLI HelloWorld Component:

<template>
  <div class="hello">
    <h1>{{ msg }}</h1>
    <p>
      For a guide and recipes on how to configure / customize this project, <br />check out the
      <a href="https://cli.vuejs.org" target="_blank" rel="noopener">vue-cli documentation</a>.
    </p>
    <h3>Installed CLI Plugins</h3>
    <ul>
      <li>
        <a
          href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-babel"
          target="_blank"
          rel="noopener">
          babel
        </a>
      </li>
      <li>
        <a
          href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-eslint"
          target="_blank"
          rel="noopener">
          eslint</a>
      </li>
    </ul>
    <h3>Essential Links</h3>
    <ul>
      <li><a href="https://vuejs.org" target="_blank" rel="noopener">Core Docs</a></li>
      <li><a href="https://forum.vuejs.org" target="_blank" rel="noopener">Forum</a></li>
      <li><a href="https://chat.vuejs.org" target="_blank" rel="noopener">Community Chat</a></li>
      <li><a href="https://twitter.com/vuejs" target="_blank" rel="noopener">Twitter</a></li>
      <li><a href="https://news.vuejs.org" target="_blank" rel="noopener">News</a></li>
    </ul>
    <h3>Ecosystem</h3>
    <ul>
      <li><a href="https://router.vuejs.org" target="_blank" rel="noopener">vue-router</a></li>
      <li><a href="https://vuex.vuejs.org" target="_blank" rel="noopener">vuex</a></li>
      <li>
        <a href="https://github.com/vuejs/vue-devtools#vue-devtools" target="_blank" rel="noopener">vue-devtools</a>
      </li>
      <li><a href="https://vue-loader.vuejs.org" target="_blank" rel="noopener">vue-loader</a></li>
      <li><a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener">awesome-vue</a></li>
    </ul>
  </div>
</template>

<script>
export default {
  name: "HelloWorld",
  props: {
    msg: String
  }
};
</script>

<!-- Add "scoped" attribute to limit CSS to this component only -->
<style scoped lang="scss">
h3 {
  margin: 40px 0 0;
}
ul {
  list-style-type: none;
  padding: 0;
}
li {
  display: inline-block;
  margin: 0 10px;
}
a {
  color: #42b983;
}
</style>

Output with Prettier:

<template>
  <div class="hello">
    <h1>{{ msg }}</h1>
    <p>
      For a guide and recipes on how to configure / customize this project, <br />check out the
      <a href="https://cli.vuejs.org" target="_blank" rel="noopener">vue-cli documentation</a>.
    </p>
    <h3>Installed CLI Plugins</h3>
    <ul>
      <li>
        <a
          href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-babel"
          target="_blank"
          rel="noopener"
        >
          babel
        </a>
      </li>
      <li>
        <a
          href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-eslint"
          target="_blank"
          rel="noopener"
          >eslint</a
        >
      </li>
    </ul>
    <h3>Essential Links</h3>
    <ul>
      <li><a href="https://vuejs.org" target="_blank" rel="noopener">Core Docs</a></li>
      <li><a href="https://forum.vuejs.org" target="_blank" rel="noopener">Forum</a></li>
      <li><a href="https://chat.vuejs.org" target="_blank" rel="noopener">Community Chat</a></li>
      <li><a href="https://twitter.com/vuejs" target="_blank" rel="noopener">Twitter</a></li>
      <li><a href="https://news.vuejs.org" target="_blank" rel="noopener">News</a></li>
    </ul>
    <h3>Ecosystem</h3>
    <ul>
      <li><a href="https://router.vuejs.org" target="_blank" rel="noopener">vue-router</a></li>
      <li><a href="https://vuex.vuejs.org" target="_blank" rel="noopener">vuex</a></li>
      <li>
        <a href="https://github.com/vuejs/vue-devtools#vue-devtools" target="_blank" rel="noopener"
          >vue-devtools</a
        >
      </li>
      <li><a href="https://vue-loader.vuejs.org" target="_blank" rel="noopener">vue-loader</a></li>
      <li>
        <a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener"
          >awesome-vue</a
        >
      </li>
    </ul>
  </div>
</template>

<script>
export default {
  name: 'HelloWorld',
  props: {
    msg: String,
  },
}
</script>

<!-- Add "scoped" attribute to limit CSS to this component only -->
<style scoped lang="scss">
h3 {
  margin: 40px 0 0;
}
ul {
  list-style-type: none;
  padding: 0;
}
li {
  display: inline-block;
  margin: 0 10px;
}
a {
  color: #42b983;
}
</style>

What I'm referring to:

        <a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener"
          >awesome-vue</a
        >

Hopefully this can be addressed. No clue if this is a bug or if this was intended when the support for Vue and others was added, but some discussion would be appreciated. :)

@duailibe
Copy link
Member

duailibe commented Dec 4, 2018

Hi @marbuser

If we were to respect the option, you might expect to print that as:

<a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener">
  awesome-vue
</a>

Unfortunately, unlike JSX, that line break between > and awesome-vue, and between awesome-vue and </a>, may change how your HTML is rendered.

So we print it like this to preserve >awesome-vue< without line breaks. See https://prettier.io/docs/en/options.html#html-whitespace-sensitivity and https://prettier.io/blog/2018/11/07/1.15.0.html#whitespace-sensitive-formatting for more information on the subject.

@duailibe duailibe added the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Dec 4, 2018
@marbuser
Copy link
Author

marbuser commented Dec 4, 2018

Hi @marbuser

If we were to respect the option, you might expect to print that as:

<a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener">
  awesome-vue
</a>

Unfortunately, unlike JSX, that line break between > and awesome-vue, and between awesome-vue and </a>, may change how your HTML is rendered.

So we print it like this to preserve >awesome-vue< without line breaks. See https://prettier.io/docs/en/options.html#html-whitespace-sensitivity and https://prettier.io/blog/2018/11/07/1.15.0.html#whitespace-sensitive-formatting for more information on the subject.

Hey duailibe,

Thanks for the quick reply.
I was aware of the HTML whitespace options, but from what I've experimented, I still got the same results. I thought setting it to 'ignore' would cause it to result in what I wanted but not so much.

Is it possible to perhaps disable this bracket functionality in Prettier then and combine this with another option then like eslint to solve this?

If not, then is the option really to just stick with it? Because from what I tested in the playground going through all the options, there doesn't really seem to be a way to disable it. On all html options it will put the brackets on new line. :/

Thanks! :)

@no-response no-response bot removed the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Dec 4, 2018
@duailibe
Copy link
Member

duailibe commented Dec 4, 2018

I thought setting it to 'ignore' would cause it to result in what I wanted but not so much.

The ignore will just make it possible to insert a new line before/after tags:

Prettier 1.15.3
Playground link

--html-whitespace-sensitivity ignore
--parser html

Input:

<a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener">awesome-vue</a>

Output:

<a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener">
  awesome-vue
</a>

But it won't change cases where we have to break "inside" the tag. For example, we will print:

<a
  href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-babel"
  target="_blank"
  rel="noopener"
>

instead of:

<a
  href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-babel"
  target="_blank"
  rel="noopener">

Is that what you want? There's no way do that currently

@marbuser
Copy link
Author

marbuser commented Dec 4, 2018

I thought setting it to 'ignore' would cause it to result in what I wanted but not so much.

The ignore will just make it possible to insert a new line before/after tags:

Prettier 1.15.3
Playground link

--html-whitespace-sensitivity ignore
--parser html

Input:

<a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener">awesome-vue</a>

Output:

<a href="https://github.com/vuejs/awesome-vue" target="_blank" rel="noopener">
  awesome-vue
</a>

But it won't change cases where we have to break "inside" the tag. For example, we will print:

<a
  href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-babel"
  target="_blank"
  rel="noopener"
>

instead of:

<a
  href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-babel"
  target="_blank"
  rel="noopener">

Is that what you want? There's no way do that currently

Hey duailibe,

Yeah this is the sort of output I was hoping for;

<a
  href="https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-babel"
  target="_blank"
  rel="noopener">

No matter though. If it can't be done, it can't be done I guess.

Are there any current plans/discussions about perhaps figuring out a way to implement something this way?

Thanks

@duailibe
Copy link
Member

duailibe commented Dec 4, 2018

I don't think this was requested before (but I could be wrong), so there's currently no discussion for this feature.

cc @ikatyang

@duailibe duailibe added status:needs discussion Issues needing discussion and a decision to be made before action can be taken type:option request Issues requesting a new option. We generally don’t accept these unless there is technical necessity. lang:html Issues affecting HTML (and SVG but not JSX) labels Dec 4, 2018
@ikatyang
Copy link
Member

ikatyang commented Dec 5, 2018

This looks like a duplicate of #5377, let's track the issue there.

Note that it's only possible to put > on the same line if the whitespace there is not sensitive or there's already a whitespace.

@ikatyang ikatyang closed this as completed Dec 5, 2018
@ikatyang ikatyang added type:duplicate Issues that are a duplicate of a previous issue and removed status:needs discussion Issues needing discussion and a decision to be made before action can be taken type:option request Issues requesting a new option. We generally don’t accept these unless there is technical necessity. labels Dec 5, 2018
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Mar 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:html Issues affecting HTML (and SVG but not JSX) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:duplicate Issues that are a duplicate of a previous issue
Projects
None yet
Development

No branches or pull requests

3 participants