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

Add lit-analyzer, prettier and add missing closing tag #307

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Misiu
Copy link
Contributor

@Misiu Misiu commented Sep 22, 2022

I noticed that device-card.ts had a missing div tag, this adds it, hopefully in the right place.
Btw how to test web.esphome on dev machine?

I've also added additional tasks to package.json, they will allow formatting code and finding problems in lit (currently 20 problems in 11 files).

@balloob
Copy link
Member

balloob commented Sep 23, 2022

Test web with running ./web.esphome.io/script/develop_web

Comment on lines +11 to +14
"lint:prettier": "prettier \"src/**/*.{js,ts,json,css,md}\" --check",
"format:prettier": "prettier \"src/**/*.{js,ts,json,css,md}\" --write",
"lint:types": "tsc",
"lint:lit": "lit-analyzer \"src/**/*.ts\""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see much need for these commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea was to mimic actions that we have in HA Frontend and maybe add them in pre-commit. Especially prettier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an ideal world, we have a pre-commit, and then run pre-commit in check mode in CI. Pre-commit is Python based, not sure if there is a JS variant or that we require people to have Python? (not the end of the world)

But if we do pre-commit, would we still need the tasks here? And we shouldn't add tasks unless we have pre-commit in place

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's doable for JS too, using husky, I think is't already done in frontend: https://github.com/home-assistant/frontend/blob/dev/package.json#L19
Quick search: https://blog.nerdjfpb.com/how-to-add-eslint-prettier-and-husky-git-hook-in-react-js-2022/, https://www.stxnext.com/blog/eslint-prettier-husky/

So we should do ESLint, prettier, and lit-analyzer in pre-commit?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, let's mimic HA frontend 👍

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.

2 participants