Limit rules to templates tagged with lit-html's html tag

[balloob]

I'm introducing Lit Element into my Polymer 3 project. I won't be able to convert all elements to Lit from the get go and so have a mix of Polymer 3 and Lit Elements.

The issue is that the rules will scan based on finding a template literal expression started with a html tag, however that is also how Polymer 3 functions (example.

A fix for this would be to add a check that the html tag that is used was imported from the lit-html package (lit-element re-exports html from lit-html).

[43081j]

Maybe this should only apply to a subset of the rules.

Many of them can apply to both polymer and lit (such as invalid-html, etc).

It is probably best we make the lit-specific ones check that they are imported from lit, if possible. So:

• lit/no-legacy-template-syntax
• lit/binding-positions
• lit/no-property-change-update
• lit/attribute-value-entities

Probably these are lit-only.

[LarsDenBakker]

There are other base classes which use lit html, so its a bit tricky.

[balloob]

The other base classes will still use html exported from lit-html right?

[LarsDenBakker]

Ah, right. Yeah, though can you follow the export chain to lit-html? In our case our base class re-exports the html tag from lit-html. LitElement does that too.

[43081j]

ESLint is meant to operate on a per-file basis, we should be avoiding trying to traverse ASTs of other files (as it means you couldn't eslint somefile.js by its self).

So while this could be achieved, it may have to be "best effort":

• check if html came from lit-element or lit-html
• if it didn't, don't apply the rule
• provide an option in the rule to always enable it rather than only for the lit packages

[LarsDenBakker]

That makes sense. I would revert the default personally :)

[coreyfarrell]

Some of these rules apply to more than just lit-html, any module which processes HTML in JS templates. I understand that lit-html is the primary target but I just want to point out that these rules are useful for other systems. I'm using plugin:lit/recommended to check lighterhtml templates, the only rule I have to disable is lit/no-invalid-html (lighterhtml allows self-closing tags).

[stramel]

Might make sense to have a setting for excluding/including certain modules where html came from?

[coreyfarrell]

eslint supports overriding rules for specific file globs, maybe this module could make it easier to use that functionality, support something like:

{
  "extends": ["plugin:lit/recommended"],
  "env": {"browser": true},
  "overrides": [
    {
      "files": ["html/polymer-component-1.js", "html/legacy/**/*.js"],
      "extends": ["plugin:lit/recommended-polymer"]
    }
  ]
}

Where the recommended-polymer settings would not just enable rules that apply to polymer but also turn off rules from the normal recommended which do not apply. This would create a reasonably easy way for projects migrating from polymer to support the time where both are in use without adding any configuration burden for projects that are either using lit-html indirectly or using something else that is similar to lit-html.

https://eslint.org/docs/user-guide/configuring#configuration-based-on-glob-patterns

[steverep]

Just wanted to point out that this issue can adversely affect actual lit templates as well. Not considering the import of the tag and only considering fixed names means that a simple rename inadvertently disables the plugin:

import { html as myTag } from "lit";

// Not linted
const myTemplate = myTag`<>`;

[43081j]

yep it may be that we just need to be more explicit about where html comes from (lit, lit-html, lit-element).

originally we wanted to avoid that so people could import html from wherever (another file, another library, etc). but i imagine thats a very extreme edge case

[steverep]

Why not just let users configure how they want?

      settings: {
        "lit/modules": {
          lit: ["html", "svg", "css"],
          "@polymer/polymer/lib/utils/html-tag": ["html"],
        },
      },

[43081j]

we could, eslint-plugin-wc does similar for whitelisted base classes

allowing it to be strict against an import path would be good. but a start would be just allowing control of the const names

[steverep]

allowing it to be strict against an import path would be good. but a start would be just allowing control of the const names

I guess that would help, but then it's too easy IMO to create a template that doesn't get linted by using a name that's not in the settings.

Specifying the module identifiers and the actual tag names imported from them is much more fixed and thus less prone to error.