search

adobe / aem-spa-project-archetype

Maven Archetype for creating new AEM SPA projects
Apache License 2.0
61 stars 32 forks source link

[Angular] Extracting all MapTo and EditConfigs into a single file is an anti-pattern #54

Closed davidjgonzalez closed 4 years ago

davidjgonzalez commented 5 years ago

https://github.com/adobe/aem-spa-project-archetype/blob/4ce2eaa02d49cedef6e68a29a5f613360862d9a0/src/main/resources/archetype-resources/angular-app/src/app/components/mapping.ts#L17

Extracting all EditConfigs and MapTo's into a single mappings.ts is confusing and breaks the separation of concerns (the concerns for a single component are both split up, and then partially consolidated into a single monolithic file). Ideally, each Component would define its own EditConfig as well as MapTo.

pfauchere commented 5 years ago

Thank you @davidjgonzalez. The proposition makes total sense. It has been a debate for some time among the developers and we couldn't decide so we deferred the uniformization. I am personally also in favor of having the mapping occur either in the package or the file that contains the component class declaration

fistakus commented 5 years ago

+1 but for a bit different reason.

The binding:

MapTo('${projectName}/components/text')(TextComponent, TextEditConfig);

is implicitly defining the data model (props) what would be injected into the Component. So, this section of the TextComponent:

export class TextComponent {
  @Input() richText: boolean;
  @Input() text: string;
  @Input() itemName:string;

has to be revised every time the data model type: '${projectName}/components/text' is changed.

Having both in a single file makes this connection more visible and any potential issue easier to spot (i.e. during Code Reviews).