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

Media queries ignore printWidth #6729

Open
algomaster99 opened this issue Oct 28, 2019 · 12 comments
Open

Media queries ignore printWidth #6729

algomaster99 opened this issue Oct 28, 2019 · 12 comments
Labels
lang:css/scss/less Issues affecting CSS, Less or SCSS status:needs discussion Issues needing discussion and a decision to be made before action can be taken type:bug Issues identifying ugly output, or a defect in the program

Comments

@algomaster99
Copy link

Prettier 1.18.2
Playground link

Scenario 1

Input:

<A
  a={`
	${b} is a constant. ${b} is a constant.${b} is a constant.${b} is a constant.${b} is a constant.
    ${b} is a constant. 
  `}
>
</A>

Output:

<A
  a={`
	${b} is a constant. ${b} is a constant.${b} is a constant.${b} is a constant.${b} is a constant.
    ${b} is a constant. 
  `}
></A>

Expected behavior:
As expected.

Scenario 2

Input:

const Graphic = styled.section`
  @media only screen and
  (min-device-width: 768px) and
  (max-device-width: 1024px) {
    overflow-x: scroll;
    overflow-y: hidden;
  }
`

Output:

const Graphic = styled.section`
  @media only screen and (min-device-width: 768px) and (max-device-width: 1024px) {
    overflow-x: scroll;
    overflow-y: hidden;
  }
`

Expected behavior:
Should have left it multi-line

Why is there an inconsistent breaking of template literals? When does it break and when not? Referring to this PR - #5979, what is meant by "simple template literals"?

@thorn0
Copy link
Member

thorn0 commented Oct 28, 2019

Scenario 2 has nothing to do with the simple/not simple distinction. The thing is Prettier special-cases some hard-coded template literal tags, including styled.*, and applies special rules to literals that have these tags. In this case, it parses and reformats the string as CSS, using the CSS parser and printer. Other examples are (but not limited to) html`...` and graphql`...`. This feature is a bit controversial and might change (become configurable) in the next major release (see #5588 and #6626).

Did I answer your question?

@thorn0 thorn0 added the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Oct 28, 2019
@algomaster99
Copy link
Author

@thorn0 Thanks for the quick response!
My current workaround is to use "max-len": ["error", { "ignoreTemplateLiterals": true }], in eslint config to make the tests pass. Is this okay?

@no-response no-response bot removed the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Oct 28, 2019
@lydell
Copy link
Member

lydell commented Oct 28, 2019

@algomaster99 We recommend turning off all rules that conflict with Prettier. See: https://prettier.io/docs/en/integrating-with-linters.html

max-len can be used with Prettier, but I wouldn't personally recommend it. You’ll always be fighting with it. See: https://github.com/prettier/eslint-config-prettier#max-len

@algomaster99
Copy link
Author

Thanks @lydell !
But apparently, we have to use max-len. We think it will make our code look neater. Anyway, thanks for the help.

@thorn0 thorn0 added the type:question Questions and support requests. Please use Stack Overflow for them, not the issue tracker. label Oct 29, 2019
@thorn0 thorn0 closed this as completed Oct 29, 2019
@thorn0
Copy link
Member

thorn0 commented Oct 29, 2019

It's actually a bug. Media queries ignore --print-width. Can't find another issue about this, so let's reopen this one.

Prettier 1.18.2
Playground link

--parser css
--print-width 40

Input:

@media only screen  and
(min-device-width: 768px) and
(max-device-width: 1024px) {
    overflow-x: scroll;
    overflow-y: hidden;
}

Output:

@media only screen and (min-device-width: 768px) and (max-device-width: 1024px) {
  overflow-x: scroll;
  overflow-y: hidden;
}

@thorn0 thorn0 reopened this Oct 29, 2019
@thorn0 thorn0 added lang:css/scss/less Issues affecting CSS, Less or SCSS type:bug Issues identifying ugly output, or a defect in the program and removed type:question Questions and support requests. Please use Stack Overflow for them, not the issue tracker. labels Oct 29, 2019
@thorn0 thorn0 changed the title Inconsistent breaking of template literals Media queries ignore printWidth Oct 29, 2019
@alexander-akait
Copy link
Member

@thorn0 it is expected, we try to avoid break @media for better readability, i can't find discussion, but it was in some of PRs

@thorn0
Copy link
Member

thorn0 commented Oct 29, 2019

I can't find any such discussion either. Looks like a bug to me.

@alexander-akait
Copy link
Member

alexander-akait commented Oct 29, 2019

@thorn0 hm, what is expected:

@media only screen and
  (min-device-width: 768px) and
  (max-device-width: 1024px) {
    overflow-x: scroll;
    overflow-y: hidden;
}
@media only screen  
  and (min-device-width: 768px)
  and (max-device-width: 1024px) {
    overflow-x: scroll;
    overflow-y: hidden;
}

@alexander-akait
Copy link
Member

alexander-akait commented Oct 29, 2019

@thorn0 also for me it is not bug, it is feature, we don't break code in this case

@thorn0
Copy link
Member

thorn0 commented Oct 29, 2019

We might want to research how exactly people usually prefer to break media queries, but this expression can be even longer and not breaking it doesn't make sense to me.

@alexander-akait
Copy link
Member

@thorn0 usually @media doesn't have more than two and (like in example) so breaking decrease readability, from my experience with postcss, stylelint, cssnano and etc, most of developer love write @media in one line, but it’s not heuristic

@thorn0
Copy link
Member

thorn0 commented Oct 29, 2019

Let's just keep the issue open for now.

@thorn0 thorn0 added the status:needs discussion Issues needing discussion and a decision to be made before action can be taken label Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang:css/scss/less Issues affecting CSS, Less or SCSS status:needs discussion Issues needing discussion and a decision to be made before action can be taken type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests

4 participants