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

Fix single spaces in templates. #1944

Closed

Conversation

afwn90cj93201nixr2e1re
Copy link
Contributor

Fixes #1927, #1703

Proposed Changes

@codecov
Copy link

codecov bot commented Nov 2, 2019

Codecov Report

Merging #1944 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #1944   +/-   ##
=======================================
  Coverage   64.74%   64.74%           
=======================================
  Files          66       66           
  Lines        2377     2377           
  Branches      761      776   +15     
=======================================
  Hits         1539     1539           
  Misses        769      769           
  Partials       69       69

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7800a29...7761565. Read the comment docs.

@jtommy
Copy link
Member

jtommy commented Nov 3, 2019

@afwn90cj93201nixr2e1re if i looked well using withespace the compiler doesn't check preserveWhitespace attrs

@jtommy
Copy link
Member

jtommy commented Nov 3, 2019

Ok but default value is undefined, you might omit it, don't you ?

@jtommy
Copy link
Member

jtommy commented Nov 3, 2019

I mean whitespaceOption.whitespace

@jtommy
Copy link
Member

jtommy commented Nov 3, 2019

@afwn90cj93201nixr2e1re reading the TS in the future we'll able to use only 'preserve' or 'condense'

@jtommy
Copy link
Member

jtommy commented Nov 6, 2019

I should read docs because it's an important change

@afwn90cj93201nixr2e1re
Copy link
Contributor Author

What about this one @jtommy .

@Tofandel
Copy link
Contributor

Tofandel commented Aug 2, 2020

If you want to reopen this

const vuePluginConfig = {
    template: {
        isProduction: true,
        compilerOptions: {
            whitespace: 'condensed',
        }
    }
}

This is the configuration, it will not cause any issue with the build and is not a breaking change, I am using this option for all my projects as well, you may have some failing snapshots that need to be updated but that's it

Just know that if you have HTML like

<span>
  text
</span>

It will be output with the condensed option as

<span> text </span>

It is only a html compression feature and doesn't visually make any difference, only changes the output markup

So the issue will not be fixed for those cases and the eslint config should be updated to change single line components to prevent whitespaces

Those components are mostly td, i, span, strong

The doc of rollup is outdated

@afwn90cj93201nixr2e1re
Copy link
Contributor Author

He don't wanna do it.

@afwn90cj93201nixr2e1re
Copy link
Contributor Author

Just pls stop saying stuff like that without any deep knowledge.
изображение

Read docs about that before saying anything about 'cosmetic' changes, read ton's of issues where it was descrbied by Evan especially.
You shouldn't write code like that:

<span>
  text
</span>

It's not right, read vue docs and other stuff which related to compiler parsing, there's shouldn't be any useless spaces (also no useless LF/CRLF (new line symbols)) if they are not needed.

you know you can make pull requests to fix those issues as well

Boi, ofc i know,

and it doesn't mean there can't be a new major fixing those and new issues in a few months

Also pls stop saying that, jtommy doesnt making any breakable changes on 0.9.0+ releases coz it's gonna be brekable and that's why we wait for months/years just for getting fixes...

@jtommy
Copy link
Member

jtommy commented Aug 2, 2020

@afwn90cj93201nixr2e1re Make a PR if you want

@Tofandel
Copy link
Contributor

Tofandel commented Aug 2, 2020

@afwn90cj93201nixr2e1re Wow you're so rude, you should read the doc, it's exactly what I explained

@afwn90cj93201nixr2e1re
Copy link
Contributor Author

Bruh, i already made it.. you dont see that pr? You said that you gonna read docs, did you?

@afwn90cj93201nixr2e1re
Copy link
Contributor Author

#1944 (comment)

@afwn90cj93201nixr2e1re
Copy link
Contributor Author

As i said read vue develops comments before discussing things that u don't know @Tofandel.

Full vue team said that this options are useless and bo one should use them in practice.
Keep using template's, that's your choice. Im not rude, that just unpopular opinion.

@afwn90cj93201nixr2e1re
Copy link
Contributor Author

afwn90cj93201nixr2e1re commented Aug 2, 2020

@jtommy as i said in 0.9.0 pr we should remove extra lines, if you don't wanna add this options into compiler config (i agree with that). Just read review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replacing extra new lines to nothing instead single space on compilation step.
3 participants