Error for unrecognized tags

[mlrawlings]

Currently, if you have a template with the following

<some-tag-that-does-not-exist/>

The following HTML will be output:

<some-tag-that-does-not-exist></some-tag-that-does-not-exist>

We should instead throw an Error if the tag is not a standard HTML tag or has not been defined as a Marko custom tag. This will help catch typos as well as users who might accidentally write concise style tags at the root when they meant text (or tried to write JS).

[mlrawlings]

If you are using webcomponents, you will be able to list those tags in a marko.json file.

The Error message will contain helpful info. Something like

Marko does not recognize the <di> tag.  Did you mean <div>?  
If you do want to output the <di> tag as HTML, you can whitelist 
it in your marko.json file. See https://markojs.com/docs/somepage 
for more info.

[jsumners]

An error should not be thrown -- https://html.spec.whatwg.org/multipage/scripting.html#custom-elements

[patrick-steele-idem]

@jsumners see the comment from @mlrawlings above. The idea is that you would need to whitelist the custom elements by registering them in your project's marko.json:

src/marko.json:

{
  "<my-custom-element>": { /* empty */ }
}

It's a tradeoff, but I am in favor of catching problems earlier at compile-time.

[jsumners]

That just goes back to the boilerplate issue I opened a while back -- lasso-js/lasso-marko#3

Now, if the parser encounters <my-awesome-element>...</y-awesome-element> then it should certainly throw an error. But the author may be fully expecting <my-awesome-element/> to be dumped into the rendered HTML as-is.

[patrick-steele-idem]

I suppose we could come up with a way for declaring non-standard tags as being tags that should passthrough. For example:

<!my-passthrough-tag></!my-passthrough-tag>
<-my-passthrough-tag></-my-passthrough-tag>
<my-passthrough-tag custom-element></my-passthrough-tag>

I'm not saying that is a good idea, but I am just through that out there. Personally, I think registering the tag in marko.json is a reasonable tradeoff since you also get autocompletion for free and better error checking and reporting at compile-time.

[mlrawlings]

We do already throw errors for mismatched tags. This may come up more in concise mode since the tag does not need to be repeated, but also in HTML when using the self-closing syntax.

I'm not sure this is a good idea, but we could have a global "off" switch for this behavior if there are users that are heavily using Custom Elements and don't wish to list them all. If there was info about how to do that in the docs linked in the error I think that would eliminate any potential confusion.

[gilbert]

I think @jsumners has a point. It might only make sense to throw an error on a would-be concise tag.

My vote is for disabling concise mode by default (with exceptions for class, import, and export), and making it easy to enable it via a flag. That should solve most all confusion around syntax.

[Hesulan]

I'm definitely against this being the default, but I would love to have this as an opt-in setting.

[patrick-steele-idem]

I can definitely see how having many tags to whitelist would be annoying. For example a-frame has 20+ custom tags implemented as web components. But if we allowed you to whitelist patterns like <a-*> it wouldn't be too bad.

@Hesulan If we're doing this it needs to be the default as one of the big reasons for these kind of helpful error messages is to help beginners who likely won't realize there's some way to opt-in to warnings.

@mindeavor

My vote is for disabling concise mode by default

While we could put some modifier in the template (e.g. @concise), I don't like the idea that concise users would have to put boilerplate in every file. Putting that configuration in a separate file would break syntax highlight so that would not be an option.

Root level text would also be an issue if there was some global way to turn concise on or off because what would be parsed as text in HTML mode would be parsed as concise tags in concise mode.

[mlrawlings]

I'm not convinced this is the right idea, but we could also only have the warning for tags names without dashes in their name. Custom Elements require hyphens in their names so we wouldn't catch those tags with this rule.

[patrick-steele-idem]

When building a complex project you will often need to rename a UI component. It would be really nice if you could recompile all of your templates to figure out exactly which UI components broke as a result of the name change. I'm strongly in favor of throwing an error when the Marko detects an unrecognized tag as long as we get the error message perfect (the developer should know exactly what to do to register the tag and it should be super simple).

[gilbert]

I don't like the idea that concise users would have to put boilerplate in every file

Sorry, I meant a global flag. Something like require('marko/compiler').config({ conciseSyntax: true })

[patrick-steele-idem]

Sorry, I meant a global flag. Something like require('marko/compiler').config({ conciseSyntax: true })

That won't work because the syntax highlighter would not have that context and do syntax highlighting independently.

[gilbert]

Can there not be two highlighter definitions? Sublime supports it, at least. Someone enabling conciseSyntax: true will know that they're opting into extra syntax, and that they'll want to change to the relevant highlighter if they want those niceties.

[ramses0]

Whitelisting tags is decent, but whitelisting tags b/c hopefully someday somebody will use concise-mode is making the wrong tradeoff in my mind. Disable concise-mode by default. All (most) issues with top-level tags magically go away (eg: the following gem ex0007.marko). Tags are tags. <for> is <for> (and is wrapped in a tag, not a bareword). It is a serious struggle to get behind "floating class statements" as is. (specifically referring to single-file-components).

$ cat ex0007.marko
this is
for
you
<h1>Test</h1>

$ markoc *.marko
- Failed to compile "ex0007.marko". Error: Error: An error occurred while trying to compile template at path "/home/rames/Git/marko-v4-spa-example/src/pages/test/ex0007.marko". Error(s) in template:
1) [ex0007.marko:2:0] Invalid <for> tag. Argument is missing. Example: <for(color in colors)>

[Hesulan]

@mindeavor @ramses0 How would disabling concise mode actually solve the problem being discussed, which is whether to throw an error when an invalid tag is encountered? I understand that you don't like the syntax, but many of us do and are using it in production.

P.S.: You can use HTML syntax for the new tags, too: <class { ... } /> <static { ... } />.

[ramses0]

@Hesulan - root level text is terrible. My vote is for putting it into a <div>. It is especially terrible when "for he's a jolly good fellow" gets parsed as a <for ... tag by marko. However, "concise mode" has to treat every word in the file as if it might be a tag (or class or static or import or whatever else exists in the global marko keyword namespace). As someone who doesn't use concise mode, I see that trying to serve two masters in "single file components" is incredibly short-sighted.

The current single-file-component format has the benefit that it is backed by working code, but as an outsider / recreational user of markojs, I see it as full of traps for implementors and users, especially compared to non-single-file-components. It also appears as though you can't really opt out of single-file component mode either.

[gilbert]

@Hesulan I don't dislike concise mode per se; I only point out that supporting both concise and normal syntax at the same time is the root of all these problems. If concise were opt-in, all the pitfalls disappear (or at least be hidden behind a global flag).

[Hesulan]

@ramses0
You have a point, root level text is definitely a downside. But it's also easily solved with a -- , and aside from a "Hello World!" file, it would be difficult to find many real world use-cases.

Single-file components are still entirely optional, they're just more convenient than writing a separate component.js when working with small, simple components. The only component-related thing that I know of in v3 that won't work in v4 is <script marko-init>, which is being replaced by <static>.

I do understand preferring HTML over concise, and it's my opinion that the official documentation should try to use HTML syntax as much as possible (even though I almost always use concise).

[Hesulan]

@mindeavor I'm still unclear on how disabling concise syntax would solve the problem, or how it's at the root of it.

[gilbert]

If concise were disabled, @ramses0's examples of surprises would no longer be surprising; they would do what you'd expect in any HTML document.

this is
for
you
<h1>Test</h1>
var not_a_var = "this is just text, just like HTML"

for this is text as well!

${ "interpolated" + "value" }
$ console.log("this is JS code. Iffy, but acceptable")

I am plain text
<my-custom-element>
  No problems here
</my-custom-element>

[seangates]

+1 for throwing an error. Many complex projects rely on knowing when there is an issue with an unknown or incorrect tag.

[Hesulan]

@mindeavor I'm not sure what that has to do with throwing errors for invalid tag names, and those are all just different examples of the same thing - plain text as a root element.

@patrick-steele-idem On further thought, I think I would agree with throwing errors by default, as long as it's easy to disable and clearly documented.

[mlrawlings]

@Hesulan As clarification, the idea for this originally stemmed from the discussion we (@mindeavor, @ramses0 and I) had on how to reduce any confusion around syntax, specifically concise syntax at the root, but I think it has enough merit to stand on its own.

That it would throw an error for unintentional concise tags at the root is only one benefit.

[mlrawlings]

Related to this, Patrick and I are looking at creating a "standard" of sort that could be adopted by web component libraries to make their custom tags visible to tooling.

Marko's autocomplete in Atom was originally forked from autocomplete-html and it was extended to autocomplete not just HTML elements, but also Marko custom tags that were discovered through directory scanning and marko.json files.

We're thinking that we could submit a PR back to the original package that would support discovering an html-elements.json in project, but also at the root of any packages that the project depends on. This process and the format of the json file would be similar to the Marko process and format, which has worked well for us.

This would allow a web component package to simply add this file (which could be generated) and autocomplete would be made available to the autocomplete-html package (which has over 200k downloads). There's an immediate benefit to doing so, but future tools would also be able to make use of this information.

And of course so would Marko. If we could get the web community to adopt this, it would mean you might never (or rather, rarely) have to add definitions/exceptions for third-party custom element tags on your own.

That's definitely a big "IF", but between reaching out to other developers, possibly making some PRs to popular libraries and the fact that there's benefit to the libraries and end users (it's not purely driven by a need of Marko) it could very well work.

[austinkelleher]

Great idea. I fully support that.

[gilbert]

@Hesulan Saving people from root-level text is the whole point of this thread, isn't it? It's only a problem because of automatic concise mode.

This will help catch typos as well as users who might accidentally write concise style tags at the root when they meant text (or tried to write JS).

[Hesulan]

@mindeavor Ah, I see what you meant now. That's not the point though, the point is to catch invalid tag names. Root level text in concise mode is just one of the many ways that a typo could slip past, and to simply disable such a widely used feature by default would be a huge overreaction and wouldn't solve the problem of invalid tag names. Catching accidental root-level concise tags is only a nice side-effect of the proposed solution.

[gilbert]

@Hesulan I know the goal is to prevent typos, but the only reason this came up is because of concise mode. After all, any custom element tag is valid HTML.

In principle there should be no errors thrown, because an unknown tag is 100% valid in the browser. However, in practice I don't feel that strongly about it, and wouldn't mind if these errors made it in.

Actually, this just dawned on me – custom element tags are only valid HTML if they contain a dash within the name. Since JavaScript keywords nor identifiers can contain dashes, doesn't that mean there is only a reason to throw errors on unrecognized single-word tags?

[Hesulan]

@mindeavor From what I understood, the reason this came up is because if you type something like <ing ... /> it's quite likely that you meant to type <img ... /> unless you've explicitly defined a custom ing tag. Throwing an error would also help to catch accidental concise tags. I honestly can't remember the last time I even used root-level text outside of a Hello World example, but I can see how it would get annoying.

I do think only throwing on single-word tags is a reasonable condition, and I believe @mlrawlings mentioned something similar earlier in this thread.

[gilbert]

oops, I totally missed his comment on that :)

[mlrawlings]

I still think starting with throwing on all unrecognized tags is a sane default and the error message would point you to information on how to configure things:

Configuring in marko.json

Throw on any unrecognized tag (default value):

{
    "ignore-unrecognized-tags": false || []
}

Turn off errors/warnings:

{
    "ignore-unrecognized-tags": true || ["*"]
}

Turn off errors for tags with dashes:

{
    "ignore-unrecognized-tags": ["*-*"]
}

Turn off errors for certain tags from polymer:

{
    "ignore-unrecognized-tags": ["paper-*", "iron-*", "google-*"]
}

Defining individual tags in html-elements.json

We still need to figure out the details on this, so this probably wouldn't make it in right away.

Defining tags in html-elements.json will make it so Marko recognizes them and won't throw, but Marko's atom plugin will also pick them up and make them available for autocompletion when authoring your templates and components.

The goal would be for authors of the web components to generate this file as part of the web component package.

{
    "<paper-button>": {
        "url":"https://elements.polymer-project.org/elements/paper-badge",
        "description":"<paper-badge> is a circular text badge that is displayed on the top right corner of an element, representing a status or a notification. It will badge the anchor element specified in the for attribute, or, if that doesn't exist, centered to the parent node containing it.",
        "@for": {
            "type":"String",
            "description":"The id of the element that the badge is anchored to. This element must be a sibling of the badge.",
        },
        "@icon": {
            "type":"String",
            "description":"An iron-icon ID. When given, the badge content will use an <iron-icon> element displaying the given icon ID rather than the label text. However, the label text will still be used for accessibility purposes.",
        },
        "@label": {
            "type":"String",
            "description":"The label displayed in the badge. The label is centered, and ideally should have very few characters.",
        },
        "@target": {
            "type":"String",
            "description":"Returns the target element that this badge is anchored to. It is either the element given by the for attribute, or the immediate parent of the badge.",
        }
    }
}

[ramses0]

@mlrawlings would you be in favor of splitting out the potential discussion: "require concise mode to be explicitly opted in to"?

I believe it would require:

<class {
  onClick() { ... }
}/>

...which makes total sense to me. Or a flag to explicitly opt out of the potential bugginess / confusion of mixing concise-mode, markojs single file components, javascript, and HTML? I keep hearing -- is the magic token, but haven't seen any documentation on that. "use strict" is a precedent, @concise-mode was a strawman syntax, even @strict-html or @disable-concise would be welcome.

[gilbert]

For the record, I now believe this was the right choice. After upgrading to rc.13, marko immediately informed me about two invalid tags that I forgot to change during a rename! 😄

[patrick-steele-idem]

@mindeavor I experienced the same thing. We had some tests that were incorrectly referring to unregistered tags. I'm not very impressed with the Github Wiki system, but we decided to put details on errors in the Wiki: https://github.com/marko-js/marko/wiki/Error%3A-Unrecognized-Tag

Please let us know if you see any issues!