templating
templating copied to clipboard
aurelia
Templating creates unwanted custom elements
I am editing this issue, the original text is below.
I originally thought that the root of the problem was Aurelia creating custom elements from exports without convention nor decorator. But from comments below it was clarified that this is actually supported in some scenarios.
With this in mind, I now think that the root of the problem is that Aurelia should not register a local custom element when instantiating a dialog like so:
import { MyDialog } from 'dialogs/whatever';
dialogService.open({ viewModel: MyDialog });
This code should not result in the registration of a <my-dialog> custom element.
It is mostly harmless, but if your exported name clashes with other tags it's breaking the app.
From aurelia/dialog#283:
When loading a component (e.g. a view model for a dialog, a page to be loaded in the router, etc.), aurelia-templating falls back to creating a custom element based on the exported component name.
I think the logic for that is here: https://github.com/aurelia/templating/blob/7693dbd65e59e428e4052922920145b76240f3cf/src/module-analyzer.js#L253
Now consider what happens if I choose to name my ViewModel export class A { }. When used by a dialog or router, it will create a <a> custom element, which is of course unwanted and will trigger bugs if there are links in the template.
Expected behavior:
Only classes decorated with @customElement or following a convention (by default ending in CustomElement) should create custom elements.
We aren't changing this. This convention-based approach is fundamental to Aurelia and is one of the reasons why people choose Aurelia.
If you want to create a feature to configure Aurelia to opt-out of this, that's an option. But we can't change this all up. That would be devastatingly destructive.
@EisenbergEffect I am not sure you understood the bug correctly, or maybe I was unaware of an Aurelia option for a long time!
From the official documentation
Aurelia will take the JavaScript class name, strip CustomElement from the end, and convert it from InitCaps to dash-case for the custom element's name. [...] It is also possible to explicitly name your custom element by using the customElement decorator
I don't see a mention that classes with random names are converted to custom elements.
This is what happened in aurelia/dialog#283: a class that was not meant to be a custom element, named just "a" at runtime, was interpreted as a <a> custom element resource.
Re-posting here from https://github.com/aurelia/dialog/issues/283#issuecomment-306937846
Since it seems that this would be quite a breaking change, since the behavior is a "supported" (even if slightly discouraged) scenario -- is there a much easier (low risk) fix here?
Without breaking things, it doesn't seem like you can reasonably stop people from exporting a module that matches a dom thing -- but honestly if you do that -- you are on your own. But because in this case it isn't the user naming it, it is webpack changing the names, could the template stuff just be protected from using single-letter names? Which I would think would cover the majority (if not all) of these issues caused by webpack name shortening. OR is there way to to control the webpack name shortening to include some prefix (I have no idea of the allowable characters in a module name -- but say $) so that they would never conflict with a dom element?
User @eamodio made some good points in the original aurelia-dialog issue.
Apparently, using custom elements with no decorator and no naming convention is a thing. It was promoted in several places, including in the current docs:
This means that, if you wish, you may ignore the Aurelia naming convention for your custom elements. In the example above, we could have simply named the class SecretMessage. The custom element would still be named secret-message. Given this capability, it might be considered wise to utilize Aurelia's naming convention for custom elements or use the customElement decorator to be explicit when creating a component that is only meant to be used as a custom element and not as a standalone page.
So it's discouraged, but supported. I'm afraid making the CustomElement mandatory will break too much code to be doable.
Yet I still believe we have an important bug that more and more users of aurelia-dialog will hit when switching to Webpack. It's just that the fix might be more complicated.
As a reminder, the bug arises from the fact that aurelia-dialog instantiate a ViewModel without going through our plugin special dependencies, like so:
import { MyDialog } from 'dialogs/whatever';
dialogService.open({ viewModel: MyDialog });
Somehow, MyDialog -- or whatever the name -- ends up being a custom element, which is wrong.
Maybe an alternative fix is to only allow convention-less custom elements when going through <require>?
@jods4 but isn't it worse than that?
Somehow, MyDialog -- or whatever the name -- ends up being a custom element, which is wrong.
Because webpack shortens the name of MyDialog to be something like a? Or am I getting that wrong?
@eamodio
For me the core bug that has been there for a long time is that I don't expect a my-dialog custom element to be created in this situation.
The situation is indeed worse now that tools like Webpack rename the imports, which can create custom elements for <a>, <b>, <dd>...
But fundamentally, I don't think renaming is the cause of the issue. It just makes it worse/more visible.
Yeah, for me that makes it dramatically worse -- because it isn't self inflicted. And doubly so, because the errors it causes are so far removed from the actual problem -- we spun for the better part of a day trying to figure out what was going on (before we found the github issue about it) during our (ongoing) conversion from systemjs/jspm to webpack.
This seems quite an urgent issue since WebPack is now the default in Aurelia/Cli. I ran into the same problem twice this week and lost a couple of hours :-(
Ok, thanks to @bigopon, the problem goes away by using PLATFORM.moduleName in the DialogService.open() viewModel parameter.
This workaround for DialogService.open(), while it does work and I'm grateful for that, is outright horrible. I have to make a massive app wide refactor that can't be easily automated with sed/awk and throws away type safety, forcing me to validate that every single dialog in my app still works. All thanks to a 4 year old bug that has had no effort put in. Sigh 💩
@sflanker tou can refactor eitber in the user of the view model, or the view model itself by giving it an explicit name via a @customElement()
@bigopon So if I simply explicitly decorate every dialog viewModel type, I can safely continue to references these view models using the type name imported using an ES style import? That is good news 🙏
Just to clarify to be sure I understand the workaround:
// view that uses a dialog
import MyDialog from './my-dialog';
export default class MyPage {
// ...
public void showMyDialog() {
this._dialogService.open({
viewModel: MyDialog
});
}
}
// my-dialog.ts
@customElement('my-dailog') // <<--- this is the critical thing that prevents the stack overflow, need even though I never reference this with <my-dialog></my-dialog>
export default class MyDialog {
}