passing @arguments to regular html elements doesn't give a great error message

[fivetanley]

Twiddle: https://ember-twiddle.com/83bd6cc98d5038094a80cc21c11c98cb?openFiles=templates.application%5C.hbs%2C
Ember Version: 3.17.0

Tried searching for this one but apologies if I couldn't find it.

Given:

<div @someProperty={{action "foo"}}></div>

The following error appears at runtime:

jquery.js:3827 Uncaught TypeError: func is not a function
    at StatementCompilers.compile (VM32 ember.debug.js:43121)
    at compileStatements (VM32 ember.debug.js:44323)
    at maybeCompile (VM32 ember.debug.js:44312)
    at CompilableTemplateImpl.compile (VM32 ember.debug.js:44292)
    at Object.evaluate (VM32 ember.debug.js:51011)
    at AppendOpcodes.evaluate (VM32 ember.debug.js:49382)
    at LowLevelVM.evaluateSyscall (VM32 ember.debug.js:52284)
    at LowLevelVM.evaluateInner (VM32 ember.debug.js:52240)
    at LowLevelVM.evaluateOuter (VM32 ember.debug.js:52232)
    at JitVM.next (VM32 ember.debug.js:53175)

Would it be possible to give a better error message at runtime like @arguments are not allowed on regular html elements ?
Would it be possible for this to be an error at compile time?

[rwjblue]

I definitely think that the error should be much better here.

[fivetanley]

@rwjblue is this the kind of thing we would want to implement at the template compiler level in glimmer, or is it intentionally undefined behavior to have arguments passed to regular html nodes in glimmer to open up future possibilities?

[chancancode]

I think fixing it in the compiler seems good. I think the code was just too dynamic (ie TypeScript didn’t help) and so this combination was missed. Don’t think it was intentional. That said if it’s easier to do it in ember as an ast plug-in, or because we have more information (file name and stuff) that’s fine too.

[deanpapastrat]

We recently had an outage in one of our sites at Square since we had an @size attribute accidentally attached to an "a" tag in a page header in what happened to be an untested code path during a migration, and it caused the entire site to fail to render. It was near impossible for us to figure out since there's no description of what's failing in a way that Sentry could useful data for: just Cannot read property 'syscall' of null.

Is there any way we could make this warn in production, and not break the entire render cycle? In its current state, it's dangerous; at the end of day, everyone will accidentally write bugs or mis-type things, and this is a pretty harsh penalty to pay.

This was on ember-source version 3.18.1

[deanpapastrat]

And, added a PR for a lint rule to prevent this at lint time, which should help other people encountering this in the future: ember-template-lint/ember-template-lint#1332

[chancancode]

IMO, we should just fix it as a compile-time syntax error (either in glimmer-vm or in ember)

[rwjblue]

Yep, agreed. Doesn't hurt to land the template lint rule also, but it is just a stop gap for fixing it in the VM.

[fivetanley]

opened glimmerjs/glimmer-vm#1102 to help address this

[rwjblue]

(reopened to track landing the fix in Ember)