Skip to content
This repository has been archived by the owner on Jul 27, 2024. It is now read-only.

Opinionated HTML rules #78

Closed
charlespwd opened this issue Dec 12, 2020 · 2 comments · Fixed by #146
Closed

Opinionated HTML rules #78

charlespwd opened this issue Dec 12, 2020 · 2 comments · Fixed by #146

Comments

@charlespwd
Copy link
Contributor

As I understand it, the AST doesn't really recognize the HTML as HTML. Am I wrong here? Looks like there would be some gain to be had by linting HTML as well. But might be complicated.

What I'd really like is some opinionated checks that prevent stuff like:

  • <script> without `async` or `defer`
  • without srcset
  • ...?

But really it's mostly about <script> without async or defer. That should not exist. And theme-check seems like a logical place to suggest/enforce this.

Also perhaps an opportunity to add magic comments to disable rules? À la // eslint-disable-line camel-case

@macournoyer
Copy link
Contributor

As I understand it, the AST doesn't really recognize the HTML as HTML. Am I wrong here?

Correct. In the AST, an HTML fragment is just a string.

However, Yishu recently introduced an HTML parser, to lint HTML inside translations: https://github.com/Shopify/theme-check/blob/master/lib/theme_check/checks/valid_html_translation.rb#L23-L31

I think we could introduce a new type of check: HtmlCheck? But I'm just worried we'll be opening a can of worms here... So many things to lint for in HTML...

How do we differentiate between HTML rules we should implement, and those we would leave to other existing HTML linters?

<script> without async or defer. That should not exist. And theme-check seems like a logical place to suggest/enforce this.

What if we introduced a better version of the script_tag filter? And implement a check to recommend it?

defered_script_tag or/and async_script_tag?

Then implement a check to deprecate script_tag.

magic comments to disable rules?

Oh yeah, we need this! I'll create an issue.

@charlespwd
Copy link
Contributor Author

What if we introduced a better version of the script_tag filter? And implement a check to recommend it?

Hmm yup! Had in mind to add support for defer and async attributes on the script_tag + recommend these.

One annoying thing here: As soon as we give two options, users might obsess over which one is better when it doesn't matter as much as putting one of either.

Considering suggesting defer since it behaves a bit like the default script tag (it maintains execution order). If you know async is better, the rule won't tell you to change anything. But not having defer or async would be a rule break.

Would suggest of type :error.

charlespwd added a commit that referenced this issue Feb 8, 2021
Say goodbye to slow sites. Adds friction to under performing HTML.

Fixes #78
charlespwd added a commit that referenced this issue Feb 11, 2021
Say goodbye to slow sites. Adds friction to under performing HTML.

Fixes #78
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants