posthog icon indicating copy to clipboard operation
posthog copied to clipboard

chore: add html validation to Cypress

Open pauldambra opened this issue 3 years ago • 6 comments

Adds HTML validation to the after each of every Cypress test

Instructions in the dependency don't work. We get error: Argument of type 'PluginEvents' is not assignable to parameter of type '(action: string, arg: Record<string, Function>) => void'.\n Types of parameters 'action' and 'action' are incompatible.\n Type 'string' is not assignable to type '\"after:run\"'."

Screenshot 2022-09-07 at 12 29 25

pauldambra avatar Sep 07 '22 11:09 pauldambra

Logged with plugin author here https://gitlab.com/html-validate/cypress-html-validate/-/issues/16

pauldambra avatar Sep 07 '22 11:09 pauldambra

Despite the TS error the test will run. Way too many errors to run on after each.

93 errors on the home page:

require-sriSRI "integrity" attribute is required on <script> element
7
script-type"type" attribute is unnecessary for javascript resources
8
require-sriSRI "integrity" attribute is required on <script> element
9
script-type"type" attribute is unnecessary for javascript resources
10
require-sriSRI "integrity" attribute is required on <link> element
11
require-sriSRI "integrity" attribute is required on <link> element
12
require-sriSRI "integrity" attribute is required on <link> element
13
require-sriSRI "integrity" attribute is required on <link> element
14
require-sriSRI "integrity" attribute is required on <link> element
15
require-sriSRI "integrity" attribute is required on <link> element
16
script-type"type" attribute is unnecessary for javascript resources
17
require-sriSRI "integrity" attribute is required on <link> element
18
element-required-attributes<button> is missing required "type" attribute
19
wcag/h30Anchor link must have a text describing its purpose
20
input-missing-label<input> element does not have a <label>
21
element-permitted-content<div> element is not permitted as content under <span>
22
element-permitted-content<div> element is not permitted as content under <ul>
23
element-permitted-content<div> element is not permitted as content under <ul>
24
wcag/h30Anchor link must have a text describing its purpose
25
element-permitted-content<div> element is not permitted as content under <ul>
26
element-permitted-content<div> element is not permitted as content under <ul>
27
text-content<button> must have accessible text
28
heading-levelHeading level can only increase by one, expected <h2> but got <h3>
29
element-permitted-content<div> element is not permitted as content under <span>
30
element-permitted-content<div> element is not permitted as content under <span>
31
element-permitted-content<div> element is not permitted as content under <span>
32
element-permitted-content<div> element is not permitted as content under <span>
33
element-permitted-content<div> element is not permitted as content under <span>
34
element-permitted-content<div> element is not permitted as content under <span>
35
element-permitted-content<div> element is not permitted as content under <span>
36
element-permitted-content<div> element is not permitted as content under <span>
37
element-permitted-content<div> element is not permitted as content under <span>
38
element-permitted-content<div> element is not permitted as content under <span>
39
empty-heading<h3> cannot be empty, must have text content
40
empty-heading<h3> cannot be empty, must have text content
41
empty-heading<h3> cannot be empty, must have text content
42
empty-heading<h3> cannot be empty, must have text content
43
empty-heading<h3> cannot be empty, must have text content
44
empty-heading<h3> cannot be empty, must have text content
45
element-required-attributes<th> is missing required "scope" attribute
46
element-required-attributes<th> is missing required "scope" attribute
47
element-required-attributes<th> is missing required "scope" attribute
48
element-required-attributes<th> is missing required "scope" attribute
49
element-required-attributes<th> is missing required "scope" attribute
50
element-required-attributes<th> is missing required "scope" attribute
51
element-required-attributes<th> is missing required "scope" attribute
52
element-required-attributes<th> is missing required "scope" attribute
53
element-required-attributes<th> is missing required "scope" attribute
54
element-required-attributes<th> is missing required "scope" attribute
55
element-required-attributes<th> is missing required "scope" attribute
56
element-required-attributes<th> is missing required "scope" attribute
57
element-required-attributes<th> is missing required "scope" attribute
58
element-required-attributes<button> is missing required "type" attribute
59
text-content<button> must have accessible text
60
element-required-attributes<button> is missing required "type" attribute
61
text-content<button> must have accessible text
62
element-required-attributes<button> is missing required "type" attribute
63
text-content<button> must have accessible text
64
element-required-attributes<button> is missing required "type" attribute
65
text-content<button> must have accessible text
66
element-required-attributes<button> is missing required "type" attribute
67
text-content<button> must have accessible text
68
element-required-attributes<button> is missing required "type" attribute
69
text-content<button> must have accessible text
70
element-required-attributes<button> is missing required "type" attribute
71
text-content<button> must have accessible text
72
element-required-attributes<button> is missing required "type" attribute
73
text-content<button> must have accessible text
74
element-required-attributes<button> is missing required "type" attribute
75
text-content<button> must have accessible text
76
element-required-attributes<button> is missing required "type" attribute
77
text-content<button> must have accessible text
78
element-required-attributes<button> is missing required "type" attribute
79
text-content<button> must have accessible text
80
prefer-native-elementPrefer to use the native <progress> element
81
prefer-native-elementPrefer to use the native <progress> element
82
element-required-attributes<button> is missing required "type" attribute
83
text-content<button> must have accessible text
84
element-required-attributes<button> is missing required "type" attribute
85
text-content<button> must have accessible text
86
element-required-attributes<button> is missing required "type" attribute
87
text-content<button> must have accessible text
88
element-required-attributes<button> is missing required "type" attribute
89
text-content<button> must have accessible text
90
element-required-attributes<button> is missing required "type" attribute
91
text-content<button> must have accessible text
92
element-required-attributes<button> is missing required "type" attribute
93
text-content<button> must have accessible text
94
element-required-attributes<button> is missing required "type" attribute
95
text-content<button> must have accessible text
96
element-required-attributes<button> is missing required "type" attribute
97
text-content<button> must have accessible text
script-type"type" attribute is unnecessary for javascript resources

@mariusandra @benjackwhite

pauldambra avatar Sep 07 '22 13:09 pauldambra

Hmm... Well, I'm not sure we should fix all HTML errors. Mostly they don't matter, and especially if we override the blockiness of the elements. For example, if we make a <div /> inline, I don't care if it's in a <span /> or not. I also don't care if a <script /> tag has some extra attribute, or if <button /> is missing type (it gets submit by default, which would be annoying if we had actual <form> tags).

However, we should just agree to stick to the three basic rules, and flag them in PR reviews:

  1. block elements can contain anything
  2. text elements can contain other text elements
  3. clicky elements can't contain other clicky elements

Based on this:

  • <p> can't contain <ul>, since text elements can't contain block elements
  • <button> can't contain <a href> since clicky elements can't contain other clicky elements
  • etc

mariusandra avatar Sep 08 '22 08:09 mariusandra

So the question becomes can this library be configured to only care about the three rules. Gives me the sads for humans to check markup :)

pauldambra avatar Sep 08 '22 09:09 pauldambra

Hi, author of html-validate here. I can recommend looking into using the html-validate:standard instead of html-validate:recommended preset (see preset comparison) as a base and then disable rules you do not need or want. The standard preset only validates strictly what is allowed by the actual HTML standard and none of the more cosmetic and opinionated practices. As this is for cypress it also pulls the html-validate:document preset by default which you may or may not want.

It would still flag errors such as "<div> element is not permitted as content under <span>" as that is disallowed by the HTML standard. In practice, as you say, you can use CSS to override with display and a browser wouldn't really care anyway. I would recommend sticking to the standard but I'm also biased in the question :)

As an alternative you could easily just not use the bundled HTML5 metadata and specify your own using a configuration such as:

 {
   "extends": ["html-validate:standard"],
-  "elements": ["html5"]
+  "elements": ["./elements.json"]
 }

where elements.json would define the element metadata you need:

{
  "p": {
    "permittedContent": ["@phrasing"] // allow anything marked as phrasing
   },
  "span": {
    "phrasing": true, // mark this element as phrasing
    "permittedContent": ["@phrasing"] // allow anything marked as phrasing
  }
}

It requires some effort to write and maintain thought.

You could also override the bundled (results are merged):

 {
   "extends": ["html-validate:standard"],
-  "elements": ["html5"]
+  "elements": ["html5", "./elements.json"]
 }

Let me know if you need any assistance and don't hesitate to file features requests if you feel there is something technical preventing you from using the library.

ext avatar Sep 14 '22 00:09 ext

Hey @ext thank you!

Now getting

Your configFile is invalid: /home/runner/work/posthog/posthog/cypress.e2e.config.ts

It threw an error when required, check the stack trace below:

/home/runner/work/posthog/posthog/node_modules/cypress-html-validate/dist/plugin.js:266
  return (0, import_deepmerge.default)(a, b ?? {}, { arrayMerge: overwriteMerge });

I'm sure this is me being a fool :)

pauldambra avatar Sep 14 '22 11:09 pauldambra

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

posthog-bot avatar Nov 28 '22 07:11 posthog-bot

Hot take: never in my career has "invalid html" been the cause of any problem. Let's not add more friction to cypress 😄

mariusandra avatar Nov 29 '22 08:11 mariusandra