Restyle Migrate schema browser to react

[restyled-io]

Automated style fixes for #4432, created by Restyled.

The following restylers made fixes:

• clang-format
• prettier

To incorporate these changes, merge this Pull Request into the original. We
recommend using the Squash or Rebase strategies.

NOTE: As work continues on the original Pull Request, this process will
re-run and update (force-push) this Pull Request with updated style fixes as
necessary. If the style is fixed manually at any point (i.e. this process finds
no fixes to make), this Pull Request will be closed automatically.

Sorry if this was unexpected. To disable it, see our documentation.

[kravets-levko]

@arikfr Seems bot ignores our eslint rules, or after last updates they were dramatically changed. These restyles files are completely different from the rest of codebase

[kravets-levko]

here ^ (and other places) - double quotes

[kravets-levko]

here ^

[kravets-levko]

here ^ (and few similar places) - braces were required when arrow function has body (?)

[gabrieldutra]

@kravets-levko I think the eslint rules were changed in #4423 and this repo is just not in sync with it yet.

[kravets-levko]

@gabrieldutra I'll catch up with master in few minutes, and then will try once more (I hope bot will update this PR 😁)

[arikfr]

I suggest rebasing with master and checking this again. Some rules indeed changed in my eslint PR.

…

On Wed, Dec 11, 2019, 18:35 Gabriel Dutra ***@***.***> wrote: @kravets-levko <https://github.com/kravets-levko> I think the eslint rules were changed in #4423 <#4423> and this repo is just not in sync with it yet. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4436?email_source=notifications&email_token=AAAROLBPTS65NK6LI54YK3DQYEJFZA5CNFSM4JZRTKDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGTYQGI#issuecomment-564627481>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAROLGALMFKXYI4T5VBL7LQYEJFZANCNFSM4JZRTKDA> .

[kravets-levko]

He-he, seems bot isn't going to update this PR 🤔

[gabrieldutra]

Looks like it failed -> https://restyled.io/gh/getredash/repos/redash/jobs/148781

[gabrieldutra]

@kravets-levko fixed 👍. Already synchronized #4432 to test.

Notes:

• It updates PRs as expected
• There is an auto-added "Skip CI" label that can be interesting to use
• it doesn't seem to trigger on "Draft" PRs

[kravets-levko]

@arikfr @gabrieldutra I dumped an effective eslint config, and seems now it's much less strict as it was. For example, now it allows both single and double quotes. I'm not going to start a discussion about which are better, but IMO in any case we should prefer a single style. Also, Restyled made a strange decision - while both single and double quotes are allowed, it replaced all single quotes with double. It feels wrong, especially considering that two years ago we were doing the opposite change.

[arikfr]

For example, now it allows both single and double quotes.

This is because we are no longer enforcing style with eslint, but rather with Prettier. I don't think eslint should enforce code style, as this can be automatically fixed by Prettier.

Also, Restyled made a strange decision - while both single and double quotes are allowed, it replaced all single quotes with double.

This comes from Prettier.

It feels wrong, especially considering that two years ago we were doing the opposite change.

We can update Prettier configuration to use single quotes, but requires overriding a default. As I didn't see any difference in one or the other, thought we should aim for keeping this as default as possible. But if there is a preference, I don't mind switching updating Prettier's config to single quote.

[kravets-levko]

I think eslint configuration should match prettier's one, otherwise it does not make much sense. Eslint should signal about style issues before pretties will fix them (also, it will reduce amount of PRs). Both tools are here for the same purpose, but eslint points to code style issues (and some IDEs will use it to highlight that places while you edit a file), and prettier fixes code post factum, so developer might not know that something is wrong until they create a PR - not so suitable, isn't it?

[kravets-levko]

Also, maybe it's a good idea to run prettier at once for all codebase?

[arikfr]

Also, maybe it's a good idea to run prettier at once for all codebase?

This is exactly what I did in: #4433.

I think eslint configuration should match prettier's one, otherwise it does not make much sense. Eslint should signal about style issues before pretties will fix them (also, it will reduce amount of PRs).

The idea is that most people will just use Prettier by default before even committing the code. In case they haven't we have a fallback in form of Restyled, which will fix it for them.

I don't see a reason people should worry about seeing these warnings when it can be fixed automatically. Before messing up my setup, I just had Prettier run on save. IntelliJ has support for it too.

[kravets-levko]

I know that IntelliJ IDEA has a Prettier support, I already installed a plugin. But even in this case it's not a replacement for eslint: prettier is running manually or when saving a file, while eslint is running immediately when I type and IDEA highlights eslint errors in editor. So how it looks: I edit code, I see no any errors, but when I save it - it's re-formatted. Maybe I'm too used to eslint integration in IDEA 🙂

[arikfr]

while eslint is running immediately when I type and IDEA highlights eslint errors in editor.

Why do you care about these messages if saving the file will make them go away and they have zero impact on functionality? Let the machine do its thing and worry about the things a human is needed for. :-)

[kravets-levko]

Why do you care about these messages if saving the file will make them go away and they have zero impact on functionality?

Not sure. Sometimes I write a code in a way I think it could be more readable. For example - fragments like this (such code is quite common in our codebase):

const refreshToken = Math.random().toString(36).substr(2);

Prettier sees chaining calls and moves each of them to new line:

const refreshToken = Math.random()
  .toString(36)
  .substr(2);

And for me it's a bit annoying when I just typed such fragment, saved it and it immediately changes under my hands. It feels rude; I prefer when my editor tells me where things may be improved, and let me to decide what to do. And will auto-fix code only when it's critical. Example above shows that code before auto-formatting looked better: it was readable (maybe, even more readable than after formatting), and it was a single line, short enough. Instead we got three very short lines. It's also important, because our monitors (and editors) usually are wider than higher, so that two extra lines will allow to show more code and provide more context without need to scroll, but now they're occupied by two useless lines while having a lot of free space on the right side. And that's why it's really annoying when files are updated on save - because a moment ago I saw more lines of code, had more context, I press Ctrl+S - and now I loose some context because of such (stupid - IMHO) changes.

That's just my opinion though. Not sure if it makes a lot of sense to others or just me.

[arikfr]

Style is a subjective thing. At times we might agree that something is better, while at other times we won't. The point of using an automatic formatters like Prettier, Black and gofmt is to avoid thinking about this or discussing it.

Will we like the result all the time? No. But it's a trade off worth taking to not have to have this discussion ever again and to have a consistent codestyle.

In your example I will probably agree with you that a oneline is simpler. But is it more readable? Maybe and maybe not. I could make some arguments about how some people don't have wide editors (@rauchy sometimes edits code on an 8" tablet). But that's the thing: these debates are endless and have no real conclusion.

So we avoid wasting time or thinking about it by submitting to the formatter. Give it a try. I think that after a while you will get used to it and be pleased with the result.

[kravets-levko]

I decided to remove file change watcher (which formats file on save), and will invoke prettier manually (assigned a hotkey) - seems this option will work for me 👍