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

Consistent formatting for JS files for the website #2386

Closed
Tracked by #2312
allan2 opened this issue Jan 19, 2022 · 13 comments · Fixed by #2536
Closed
Tracked by #2312

Consistent formatting for JS files for the website #2386

allan2 opened this issue Jan 19, 2022 · 13 comments · Fixed by #2536

Comments

@allan2
Copy link
Contributor

allan2 commented Jan 19, 2022

A formatting hook in the future, but what about a Prettier config for now?

I'm using this to keep the format of what I see in the current docs.

  • JS, single quotes, 4 spaces
  • HTML, 2 spaces
  • MDX, double quotes

website/.pretterrc.json

{
    "trailingComma": "es5",
    "tabWidth": 4,
    "semi": false,
    "singleQuote": true,
    "overrides": [
        {
            "files": "*.mdx",
            "options": {
                "singleQuote": false,
                "tabWidth": 2
            }
        },
        {
            "files": "*.html",
            "options": {
                "tabWidth": 2
            }
        }
    ]
}
@voidpumpkin
Copy link
Member

A lot of yew maintainers use intelij.
Does intelij have prettier?

@koalazub
Copy link

IntelliJ plugin for Prettier

IntelliJ can locate it automatically if you've installed it in your project

@voidpumpkin
Copy link
Member

voidpumpkin commented Jan 21, 2022

Some things to consider, if we go ahead with prettier:

  • probably will need npm at the root of yew folder so that people can run npm i and be able to run prettier without IDE (unless cargo has prettier as well i haven't checked)
  • add github action to pr flow to check if formatting is correct

I personally would prefer a tool we can install using cargo instead of npm + prettier

@koalazub
Copy link

@voidpumpkin I had a look to save you some time - and I could find dprint. Which looks to be a rust version of prettier that was used by Deno until a couple of years ago when they switched to their internal deno fmt.

There is also Rome Tools which are quite active and pushing hard to complete their project in rust.

Hope this info is of some help!

Side reading info on rust tools for javascript

@ranile
Copy link
Member

ranile commented Jan 21, 2022

  • probably will need npm at the root of yew folder so that people can run npm i and be able to run prettier without IDE (unless cargo has prettier as well i haven't checked)

I don't see how that's any different than running npm start for running the site locally.

  • add github action to pr flow to check if formatting is correct

Just add a npm run fmt step to build website workflow?

Ideally in the future, we should have Yew SSG build our site. In the meantime, we should just use node tools

@voidpumpkin
Copy link
Member

I don't see how that's any different than running npm start for running the site locally.

Prettier formats .md files which we have more than just in website folder
Currently we have npm only in website folder, and that why this would mean we would need to add npm to root folder.

@ranile
Copy link
Member

ranile commented Jan 22, 2022

Prettier formats .md files which we have more than just in website folder

I think it's fine if we only format .md files in website directory. Like other .md files, .toml files are also not formatted

@voidpumpkin
Copy link
Member

voidpumpkin commented Jan 22, 2022

@voidpumpkin I had a look to save you some time - and I could find dprint. Which looks to be a rust version of prettier that was used by Deno until a couple of years ago when they switched to their internal deno fmt.

There is also Rome Tools which are quite active and pushing hard to complete their project in rust.

Hope this info is of some help!

Side reading info on rust tools for javascript

I like the look of dprint, the fact its the same prettier and you can install it with cargo. (by using dprint-plugin-prettier)
I wonder if it can diverge from users just using prettier extensions in their IDEs 🤔

@ranile
Copy link
Member

ranile commented Jan 22, 2022

I wonder if it can diverge from users just using prettier extensions in their IDEs thinking

It has it's own VS Code extension. There's no plugin that I could find for IJ though, which is unfortunate.

In any case, I think we should start with prettier for website and if that's not enough, improve from there. I'll send a PR for that soon

@voidpumpkin
Copy link
Member

I wonder if it can diverge from users just using prettier extensions in their IDEs thinking

It has it's own VS Code extension. There's no plugin that I could find for IJ though, which is unfortunate.

In any case, I think we should start with prettier for website and if that's not enough, improve from there. I'll send a PR for that soon

If we do it only for website forlder then i would rather go with just prettier

My expectation was that we could format all .md files in our repo

@allan2
Copy link
Contributor Author

allan2 commented Jan 22, 2022

Would EditorConfig work for .md for the entire project? It can also specify formatting for .rs, which would come into play if someone's editor deviated from rustfmt.

@koalazub
Copy link

I wonder if it can diverge from users just using prettier extensions in their IDEs thinking

It has it's own VS Code extension. There's no plugin that I could find for IJ though, which is unfortunate.

In any case, I think we should start with prettier for website and if that's not enough, improve from there. I'll send a PR for that soon

I was able to find it in the marketplace. Looks recently updated as well?

image

@allan2
Copy link
Contributor Author

allan2 commented Jan 23, 2022

Prettier is widely used at 18 million downloads. EditorConfig at 4.5 million. Dprint is less than 100...

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

Successfully merging a pull request may close this issue.

4 participants