43081j / eslint-plugin-lit

lit-html support for ESLint
116 stars 21 forks source link

Add support for `<html>` tags in templates #115

Closed andrico1234 closed 2 years ago

andrico1234 commented 2 years ago

I've opened up a PR in open-wc's eslint-plugin-lit-a11y package to implement an eslint rule that checks to see if the <html>'slang` attribute receives a compliant value. (W3 validate against this rule, see here and here)

Unfortunately, since we're using parse5'sparseFragment function to generate the AST, the resulting object strips out the elements like <HTML>, <body>, and <head>, which is not good since we want to run validation against the <html> element.

The way I've gotten around this is by extending the template-analyzer to check if an element is an <html> element via the following function

function isRootElement(node) {
  // You can find a breakdown of this regex pattern here: https://regex101.com/r/lKJKFv/1
  return /<html([\s\S]*)<\/html>/.test(node);
}

Then inside of the template-analyzer's constructor we make the following modification:

  const html = this.templateExpressionToHtml(node);
    const options = {
      treeAdapter,
      sourceCodeLocationInfo: true,
      onParseError: err => {
        this.errors.push(err);
      },
    });
    };

    if (isRootElement(html)) {
      this._ast = parse5.parse(html, options);
    } else {
      this._ast = parse5.parseFragment(html, options);
    }

I created a PR to implement this here but this causes open-wc's template-analyzer to become out sync with the one written in eslint-plugin-lit.

@thepassle left a comment in the PR about possibly importing your template-analyzer instead of open-wc rolling their own.

We've recently had some discussions to see what the deltas between our template analyzer, and the original template analyzer are, and see if we can just import the original template analyzer instead. If we make changes here, it'll be harder to move back to the original template analyzer.

If you think it appropriate, I'd be happy to create a PR to add the isRootElement check, to enable your package, and other consumers of the template-analyzer to add lint rules that target the <html> element.

43081j commented 2 years ago

I'm away on annual leave at the min but will take a look at importing the analyzer in the openwc stuff when I'm back. Then just see which gaps need filling in and how (incl this)

andrico1234 commented 2 years ago

Hi @43081j , a friendly bump 😊

43081j commented 2 years ago

sorry has been a busy couple of weeks, i'll take a look this weekend 👍

43081j commented 2 years ago

@andrico1234 it is being tackled here: 43081j/eslint-plugin-lit#117

once that lands, then openwc can make use of it