Coder Social home page Coder Social logo

Comments (43)

swansontec avatar swansontec commented on May 1, 2024 96

I use standard.js, which is a popular no-configuration style guide & linter. They insist on a semicolon-free style, and the only rule needed to make this work is to never start a line with (, [, or `.

I would love to see standard.js and prettier be compatible one day.

from prettier.

jlongster avatar jlongster commented on May 1, 2024 18

standard is a widely excepted format for semicolon-less JavaScript. Since there's a viable option for semicolon-less formatting already, I think this project should focus on a format that includes them (and there are technical reasons for this too, it would add a bunch of complexity).

If you don't want semicolons, use standard. Or as someone else mentioned, fork this project and try to implement it yourself. I'll take a look and we can talk more then. standard has a few forks as well that tweak basic things like semicolons (semi-standard, etc), and I think it's fine if there's a fork that has that on by default. That would fix the testing problem, too -- we use snapshots, so how would we make sure both semicolons and no-semicolons works when PRs come in? Would we have to have two identical test suites that have snapshots with different options? In fact, we have other options as well, so should we have 4-5 identical test suites with different snapshots?

I think a fork is actually a viable option and helps relieve the tension between the different styles. I'm happy to work on whatever tooling/process is needed to help maintain a fork if people really want that style.

Lastly, we can also make it easy to integrate prettier in other workflow. For example this kind of works:

prettier foo.js | standard --stdin --fix

Although if you're really after just ditching semicolons, you probably just want eslint directly that removes semicolons. But that might be kind of slow. If all you want is dropping the semicolons, I wonder if you could do it as a quick post-processing step or something. I think this kind of thing can be explored outside of prettier for now.

If someone wants to explore a no-semicolons version of this, I'd love to see it. I'm happy to advocate a forked version of this that has that, or a post-processing step, or something like that.

from prettier.

bakkot avatar bakkot commented on May 1, 2024 17

It's worth mentioning that "the only rule needed to make this work" will get progressively more complex as the language evolves. For example, under the popular public fields proposals these two classes differ:

class A {
  set
  foo(x){} // setter for 'foo'
}

class B {
  set; // uninitialized field named 'set'
  foo(x){} // method named 'foo'
}

(Note that babylon currently gets these edge cases wrong in a number of exciting ways.)

Presumably a no-semi style would have you writing

class B {
  set
  ;foo(x){}
}

So, keep in mind that the complexity cost comes not just now but also in the future. (I don't think this is well understood by people using this style.)

from prettier.

jlongster avatar jlongster commented on May 1, 2024 16

I'm open to it, but it gets problematic fast. When you don't use semicolons, you run into things like this where you are forced to put a semicolon:

var x = 5

;[1, 2, 3].forEach(...)

The first time this would be parsed correctly, but we would output:

var x = 5

[1, 2, 3].forEach(...)

which is invalid JS. (at least, not what you want.) This means that in order to support semicolon-less JS we need to code in all the ASI rules so we can see where semicolons are important. I'm not sure I'm willing to take on that complexity just yet.

Feel free to play around with it. Code is powerful and you might convince me. My preference for evolving projects is to take it slowly though, and it's hard to remove things later than to add them, so let's play with it over time and see what we can do. I'd like to keep the number of options low, but this is such a divisive issue that it could be worthwhile.

from prettier.

dtinth avatar dtinth commented on May 1, 2024 16

Here’s my take for standard users: prettier-standard-formatter.

It basically takes the output from prettier and passes it into standard --fix.

An Atom package is available.

from prettier.

gaelollivier avatar gaelollivier commented on May 1, 2024 15

Have two suggestions:

  • what about making this the default (and only) behavior ? I agree configuration options should be avoided so why not considering making this the standard ?
  • how about auto-detecting depending on the existing code ? If it has semi-colons, output semi-colons, if not, do not output them ?

from prettier.

rogeliog avatar rogeliog commented on May 1, 2024 13

@jlongster If you are both accepting PRs and want this feature, I would be more than happy to help with a PR for this one.

from prettier.

MoOx avatar MoOx commented on May 1, 2024 11

Just have a simple question.
If people write

var x = "thing"

[1].forEach(...)

how do you know the person wanted to write

var x = "thing";

[1].forEach(...);

and not

var x = "thing"[1].forEach(...)

My point is "how do we know?" because if we know, we can add or prepend a semicolon, so same thing right?

from prettier.

dtinth avatar dtinth commented on May 1, 2024 10

I tried piping the result of prettier into standard --fix and the result is quite promising: prettier-standard-formatter.

// Before
var x = 5;
[1,2,3].forEach(console.log);

// After
var x = 5;
[ 1, 2, 3 ].forEach(console.log)
// Before
var x = "thing"
[1].forEach(console.log)

// After
var x = 'thing'[1].forEach(console.log)

If the user doesn’t include a semicolon in front, these two lines gets squashed into the same line, clearing any ambiguity.

from prettier.

tunnckoCore avatar tunnckoCore commented on May 1, 2024 10

Omg. "with tabs". I can't understand why complexing the tools (not talking only specifically for prettier) when there always are workarounds with other tools and existing workflow? But yea, our different styles in that community kills us.

from prettier.

K-Leon avatar K-Leon commented on May 1, 2024 7

I would love seeing this integrated - is currently the only show stopper for me.

// Edit: Standard Packe had 200k+ Downloads last Month - i think it's something that matters for a lot of people in the JS Community

from prettier.

dtinth avatar dtinth commented on May 1, 2024 7

@timdorr

Seems as simple as looking that there's a linebreak/whitespace there

However, that goes against the rules of JavaScript. A JavaScript engine will parse it as "thing"[1] no matter what. I don’t think a code formatter should treat "thing" [1] as "thing";[1] which alters the meaning of the code.

@MoOx

what if people write this by mistake

The no-unexpected-multiline ESLint rule will warn people about this 😃.

P.S. I think the implications of using a semicolon-free style has already been well studied and there are already linter rules to avoid making mistakes in both styles. I would suggest to focus the discussion more on the complexity incurred and implementation effort required.

from prettier.

jlongster avatar jlongster commented on May 1, 2024 6

Why not just leave semicolons alone, rather than adding or removing?

The AST has no notion of "semicolons" at all. And this formatter works by printing the AST, with a few exceptions where we look at the original text. Those few exceptions are very easy cases, while checking for semicolons would be far harder.

Any chance we could start small with what @TehShrike suggested?

It'd require a ton of special-casing and isn't really trivial. This formatter discourages giving the programmer control because the goal is complete consistency. The one exception is blank lines -- we will keep blank lines because that's such an important part of the code (but we will still collapse multiple blank lines into a single one).

from prettier.

MadelineRitchie avatar MadelineRitchie commented on May 1, 2024 5

For anyone finding this via Google, note that prettier supports this option now, per this PR:
#1129

from prettier.

wmertens avatar wmertens commented on May 1, 2024 4

@MoOx There is only one way that your example will be parsed (so one canonical AST), and only two ways that prettier should output it:

with semi:

var x = "thing";

[1].forEach(...);

without semi:

var x = "thing"

;[1].forEach(...)

The rule is simply: remove semi at end of line, and insert semi at beginning of line, before array literal, parens block, or template string.

from prettier.

timdorr avatar timdorr commented on May 1, 2024 4

@MoOx

how do you know the person wanted to write

Seems as simple as looking that there's a linebreak/whitespace there and assuming they intended to have those two tokens be separate. Basically:

"thing" [1] // implicit semicolon
"thing"
[1] // implicit semicolon
"thing"[1] // no implicit semicolon

from prettier.

kentcdodds avatar kentcdodds commented on May 1, 2024 4

Any chance we could start small with what @TehShrike suggested?

Why not just leave semicolons alone, rather than adding or removing?

As an option. So: addSemicolons which would default to true

Actually, this would be enough for me, as eslint --fix will remove semicolons for me anyway. So if prettier misses removing a semicolon, eslint will do it. So if I had the ability to prevent prettier from adding semicolons then I'd be good :)

from prettier.

jlongster avatar jlongster commented on May 1, 2024 4

Since we won't be actively working on this, I'm closing this issue. But whoever reads this please know: the next step is to implement it yourself and show me the code. We can see how much complexity it adds and talk about if a fork is the right option or not.

from prettier.

TehShrike avatar TehShrike commented on May 1, 2024 3

Why not just leave semicolons alone, rather than adding or removing?

from prettier.

jlongster avatar jlongster commented on May 1, 2024 3

@dtinth You might want to check with @kentcdodds as he made https://github.com/kentcdodds/prettier-eslint as well

from prettier.

MoOx avatar MoOx commented on May 1, 2024 2

There is only one way that your example will be parsed (so one canonical AST), and only two ways that prettier should output it

I know that. My idea is "what if people write this by mistake". I guess the response is "pebkac".

from prettier.

tunnckoCore avatar tunnckoCore commented on May 1, 2024 2

I'm agree with @jlongster that it is not main priority. There are existing workarounds.

@kentcdodds and @dtinth such things don't make any sense for me. Sorry but they were unnecessary. :)

I think it was just enough to think that "the Formatter should be first, the Linter should be second".

@kentcdodds, btw, side note: CI is failing because you missed eslint in deps. And don't see value for Atom or Sublime plugin. Why not just use prettier plugin and linter-eslint it works great as I already mentioned.

from prettier.

kentcdodds avatar kentcdodds commented on May 1, 2024 2

I'm pretty sure that prettier disallows comments on the same line (which is cool with me), but it'd make more sense for it to go above. In any case, discussion about that should go in a new issue. What I was trying to point out in the gif is the flash of semicolons (because prettier) and then their subsequent removal (because eslint --fix). Whereas with my plugin, things are much smoother:

plugin

Note again that the plugin's got some issues that need PRs :)

from prettier.

jlongster avatar jlongster commented on May 1, 2024 2

(We don't disallow comments on the same line, inline comments are just plain buggy right now)

from prettier.

tunnckoCore avatar tunnckoCore commented on May 1, 2024 1

Huh. I found cool thing. Because I'm moving to Atom and just using ESLint instead of Standard, i just love the combo of Linter, Linter-ESLint + eslintrc, and atom prettier :D

It is just a whole show, because I had semi: [2, always], but atom linter can autofix (on save) the fixable errors, so I'm still no writing semicolons. Between that phase prettier prittifies and changes single quotes to double quotes (eslint warns me for a milliseconds there is doublequotes) and Atom Linter (because eslintrc) converts them back to singlequotes and insert the semicolons.

There has a bit of delay between them and it can be seen what does what.

Cool. I'm moving back to semicolons because the AtomLinter.
As I know SublimeLinter don't have support for autofix.

edit: So in short. I also wanted that option, but it seems AtomLinter comes to the rescue and can be easily fixed if you use ESLint, not some "standards" or "semistandards" :)

from prettier.

dtinth avatar dtinth commented on May 1, 2024 1

@TehShrike Because this tool doesn’t care about the existing formatting of the code.

From the README

In technical terms: prettier parses your JavaScript into an AST and pretty-prints the AST, completely ignoring any of the original formatting. Say hello to completely consistent syntax!

It’s more complex to program the pretty-printer not to output a semicolon, than to make it always output one.

from prettier.

kentcdodds avatar kentcdodds commented on May 1, 2024 1

Makes sense.

giphy

from prettier.

jlongster avatar jlongster commented on May 1, 2024 1

A semicolon option is arguably just as important as bracketSpacing, quotes, tabWiths, and trailingComma option. :-D

There are criteria for options: high impact (makes it much more broadly used with existing codebases) and low bug surface area. semicolons is high impact, but is more technically complex than tweaking one or two lines, because we need to make sure to emit them when we semantically have to, etc. It's also too high impact, meaning that it would make prettier styles diverge quite a bit, where right now we can have basically a single style even if there are a few tweaks with spacing here or there.

As I keep saying elsewhere: never say never.

from prettier.

kentcdodds avatar kentcdodds commented on May 1, 2024 1

Thanks for the reference @jlongster, I had meant to add that here and forgot 😅

from prettier.

kentcdodds avatar kentcdodds commented on May 1, 2024 1

@tunnckoCore, thanks for the tip! I'd added it to my devDependencies and yarn doesn't move the dependency when you do yarn add (I should probably file a bug)... In any case, it's been fixed. Thank you.

Why not just use prettier plugin and linter-eslint it works great as I already mentioned.

Because this:

working

So instead, I can disable --fix on linter-eslint and just use prettier-eslint-atom (as soon as it gets a few kinks worked out) and I don't get that flash of formatting. I'd say it's worth the effort.

The other underlying idea behind something like prettier-eslint is as I said elsewhere:

It's more about being able to take advantage of the advanced formatting capabilities of prettier, without sacrificing the stylistic choices of people already using ESlint to autofix/format their code. Whether that be semicolons or spacing in parameter lists.

If you don't want to use it, be my guest 😄

from prettier.

rhengles avatar rhengles commented on May 1, 2024 1

@tunnckoCore

For a few reasons:

  • It's a simple change
  • It's more efficient
  • I personally believe that tabs are the correct way to indent code
  • I won't compromise for a workaround. I want it as a first-class option.

I know I can't change what the majority of the community uses, but I cannot and will not adapt into it simply because it is what the majority uses, but only if someone manages to convince me it is a technically better option.

from prettier.

wmertens avatar wmertens commented on May 1, 2024

@bakkot Yes, indeed, in this case the semicolon injection needs to happen after the keywords set and get in classes if used as class field names. Indeed, this is extra maintenance. But is it a lot of maintenance? Will there be a lot of cases like this? Personally I'm skeptical.

from prettier.

bakkot avatar bakkot commented on May 1, 2024

@wmertens Not just 'get' and 'set' but also 'static' - but not 'async'.

Off the top of my head, there's also

class A {
  a = 0; // initialized class field
  *b(){} // generator method
}

class B {
  a = 0
  *b(){} // SyntaxError
}

And that's just with this one proposal.

It's hard to predict how much more complex it'll be in the future. That's kind of my point: adding this option will add an unknown (but certainly nonzero) amount of future complexity.

from prettier.

jlongster avatar jlongster commented on May 1, 2024

Seems as simple as looking that there's a linebreak/whitespace there and assuming they intended to have those two tokens be separate. Basically:

Yes, and this is what I meant by "we'll have to start embedding ASI-style rules into the printer".

from prettier.

TehShrike avatar TehShrike commented on May 1, 2024

@dtinth but semicolons are code, too. I would expect them to be represented in the AST.

from prettier.

aaronshaf avatar aaronshaf commented on May 1, 2024

A semicolon option is arguably just as important as bracketSpacing, quotes, tabWiths, and trailingComma option. :-D

Appreciate all you do.

from prettier.

tunnckoCore avatar tunnckoCore commented on May 1, 2024

Hm. I'm new to Atom. Yea I did notice that flash and it wasn't cool, and kinda feel slower that it should. I did follow the linked issue.

So yea, maybe make sense and I'll try your plugin.

In anyway, that specific gif seems more like a prettier bug to me. Why moves the inline comment on new line? Hm.

from prettier.

tunnckoCore avatar tunnckoCore commented on May 1, 2024

In any case, discussion about that should go in a new issue.

Yea, yea, I'm agree.

from prettier.

rhengles avatar rhengles commented on May 1, 2024

Has anyone here already made a fork with a configuration option to remove semicolons? If anyone did it or will do it, I'll try to merge it in my fork. Or if someone sends a PR, even better! 😃

from prettier.

sheerun avatar sheerun commented on May 1, 2024

I'm pasting the link to my package I've created specifically to fix this issue, as this thread is the first on google when you search for "prettier semicolons": https://github.com/sheerun/prettier-standard

I hope prettier will support no-semi rule soon enough, though!

from prettier.

alexgvozden avatar alexgvozden commented on May 1, 2024

eslint.autoFixOnSave is a great option to autofix your code

from prettier.

j-f1 avatar j-f1 commented on May 1, 2024

@vjeux Should this be unlocked? The option has been added.

from prettier.

j-f1 avatar j-f1 commented on May 1, 2024

This has been added in #1129.

from prettier.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.