Throwing on custom html element data attribute

tinovyatkin commented 6 years ago

Bug Report

Marko Version: 4.9.0

Details

Expected Behavior

I'm trying to include AMP custom tag into my Marko template to be rendered 'as-is', ie, as custom HTML element.

<amp-analytics else-if(out.global.amp && tagManagerAMP)
  config="https://www.googletagmanager.com/amp.json?id=${tagManagerAMP}&gtm.url=SOURCE_URL"
  data-credentials="include">
</amp-analytics>

and html-elements.json to make Marko recognizing tag:

{
  "<amp-analytics>": {}
}

However, Marko throws with following error:

Error: Generating code for TemplateRoot node failed. Error: Error: Generating code for <body> tag (src/marko/components/page-body/template.marko:7:0) failed. Error: Error: Generating code for If node failed. Error: Error: Generating code for If node failed. Error: Error: Generating code for <amp-analytics> tag (src/marko/components/page-body/template.marko:13:0) failed. Error: Error: Invalid JavaScript identifier: data-credentials
    at module.exports (/Users/tino/Repos/engine/node_modules/raptor-util/createError.js:7:50)
    at CodeGenerator._invokeCodeGenerator (/Users/tino/Repos/engine/node_modules/marko/src/compiler/CodeGenerator.js:138:19)
    at CodeGenerator._generateCode (/Users/tino/Repos/engine/node_modules/marko/src/compiler/CodeGenerator.js:219:38)
    at CodeGenerator.generateCode (/Users/tino/Repos/engine/node_modules/marko/src/compiler/CodeGenerator.js:263:14)
    at Compiler.compile (/Users/tino/Repos/engine/node_modules/marko/src/compiler/Compiler.js:166:38)
    at _compile (/Users/tino/Repos/engine/node_modules/marko/src/compiler/index.js:99:33)
    at Object.compile (/Users/tino/Repos/engine/node_modules/marko/src/compiler/index.js:113:12)
    at doLoad (/Users/tino/Repos/engine/node_modules/marko/src/loader/index-default.js:156:45)
    at load (/Users/tino/Repos/engine/node_modules/marko/src/loader/index-default.js:35:16)
    at Object.<anonymous> (/Users/tino/Repos/engine/src/marko/airports-vip.amp.marko.js:19:26)

I've tried to change html-elements.json like this, but it doesn't helps:

{
  "<amp-analytics>": {
    "@data-credentials": {
      "type": "String"
    }
  }
}

The above code compiles fine without data-* attribute.

Is there any workaround?

jasonmacdonald commented 6 years ago

You need to tell Marko what the valid attributes for your tag are. Either specify the exact attributes you expect or use @*: "string" (also, use lower case 's' for 'string') to just accepting anything.

So either everything...

{
  "<amp-analytics>": {
     "@*": "string"
  }
}

or be specific about ALL attributes. In your last example, you are only defining what data-credentials is, but not config.

{
  "<amp-analytics>": {
      "@config": "string",
      "@data-credentials": "string"
  }
}

tinovyatkin commented 6 years ago

@jasonmacdonald neither solution work, unfortunately, resulting in the same error.

jasonmacdonald commented 6 years ago

@tinovyatkin If you can provide a simple example of your component, I can try and dig deeper to see what's going on.

tinovyatkin commented 6 years ago

@jasonmacdonald There is no component, as I write above, I have a Marko template that should output <amp-analytics> as is, i.e. simple example is:

$ const { tagManagerAMP } = input;

<amp-analytics else-if(out.global.amp && tagManagerAMP)
  config="https://www.googletagmanager.com/amp.json?id=${tagManagerAMP}&gtm.url=SOURCE_URL"
  data-credentials="include">
</amp-analytics>

How to make this template compiling?

jasonmacdonald commented 6 years ago

Ohhhhh, I see. The <amp-analytics> isn't a Marko component. Sorry, I missed that bit. Still, I just tried to add a tag like that to a test project and it worked fine using...

{
  "<amp-analytics>": {
      "@config": "string",
      "@data-credentials": "string"
  }
}

The tag was retained, as is, in the output rendered by the browser. So, there might be something else going on, it does work.

Here's a screen grab from the chrome dev-tools

screen shot 2018-03-06 at 9 42 49 am

yomed commented 6 years ago

Hi @tinovyatkin -- this is similar to the failing test case I showed here: https://github.com/marko-js/marko/pull/918

As a workaround, try adding an entry to your top level marko.json:

"<amp-analytics>": {"html": true}

tinovyatkin commented 6 years ago

@yomed Thank you, top level marko.json did the magic! Is this behavior documented anywhere?

yomed commented 6 years ago

Nice! Unfortunately, I don't think it's currently documented. @mlrawlings dug up this workaround for me when I made that PR.

tigt commented 5 years ago

This is now sorta documented at https://markojs.com/docs/marko-json/#options, but without much explanation. This issue seems like a good candidate for a Troubleshooting section.

DylanPiercey commented 4 years ago

"<amp-analytics>": {"html": true} is the correct work around and is now documented 😄. https://markojs.com/docs/marko-json/

marko-js / marko