jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.55k stars 4.02k forks source link

Brainstorming on a new Angular directory structure #6693

Closed agoncal closed 6 years ago

agoncal commented 6 years ago

Overview of the issue

Today's Angular directory structure is quite flat and it starts to bring a few challenges. We could use this issue to note the improvements that could be done.

├── entities
│   ├── country
│   │   ├── country-delete-dialog.component.html
│   │   ├── country-delete-dialog.component.ts
│   │   ├── country-detail.component.html
│   │   ├── country-detail.component.ts
│   │   ├── country-dialog.component.html
│   │   ├── country-dialog.component.ts
│   │   ├── country-popup.service.ts
│   │   ├── country.component.html
│   │   ├── country.component.ts
│   │   ├── country.model.ts
│   │   ├── country.module.ts
│   │   ├── country.route.ts
│   │   ├── country.service.ts
│   │   └── index.ts

Motivation for or Use Case

There are several problems we encounter with this flat structure.

Same entity name in several MicroServices collide in the Gateway

There is already an issue about this. Basically, we use several microservices and aggregate them into a single gateway. Some Entities are redundant (eg. Organisation, Contact, ...) and exist in several microservices (event though they don't have the same attributes). With this flat directory structure, you end up with twice the same entity. To avoid this, we need to add the AngularJSSuffix, but it's not really nice.

Module inter-connection

Today, the client model in Angular is not typed. We use the BaseEntity for relations (here, an Organisation has one Country):

export class Organisation implements BaseEntity {
    constructor(
        public id?: number,
        public name?: string,
        public country?: BaseEntity,
    ) {
    }
}

When we type this (and I hope JHipster will one day):

export class Organisation implements BaseEntity {
    constructor(
        public id?: number,
        public name?: string,
        public country?: Country,
    ) {
    }
}

We end-up with modules depending on each-other. Same for the attempt being done now about auto-complete fields for the entities. Basically, these components can be shared by any other, so modules depend on each other.

Related issues

Suggest a Fix

If you take all these constraints into consideration, it looks like the directory structure could be improved:

Sub-directory per microservice and shared directory

This could look like this:

├── billing     // microserivce
├── invoicing   // microserivce
├── scanning    // microserivce
│   ├── entities
│   │   ├── country  // notice that model and service are not there
│   │   │   ├── country-delete-dialog.component.html
│   │   │   ├── country-delete-dialog.component.ts
│   │   │   ├── country-detail.component.html
│   │   │   ├── country-detail.component.ts
│   │   │   ├── country-dialog.component.html
│   │   │   ├── country-dialog.component.ts
│   │   │   ├── country-popup.service.ts
│   │   │   ├── country.component.html
│   │   │   ├── country.component.ts
│   │   │   ├── country.module.ts
│   │   │   ├── country.route.ts
│   │   │   └── index.ts
│   │   ├── shared
│   │   │   ├── model   // every model 
│   │   │   │   ├── country.model.ts
│   │   │   │   └── organisation.model.ts
│   │   │   ├── service   // every service
│   │   │   │   ├── country.service.ts
│   │   │   │   └── organisation.service.ts
│   │   │   ├── selection  // every selection component
│   │   │   │   ├── country-selection
│   │   │   │   │   ├── country-selection.component.html
│   │   │   │   │   ├── country-selection.component.ts
│   │   │   │   │   ├── country-selection-multi.component.html
│   │   │   │   │   └── country-selection-multi.component.ts
│   │   │   │   └── organisation-selection

Some components can get quite complex, use helper classes or css. I quite like the "sub-directory per component", but that's another step further, not sure it's really needed

├── scanning
│   ├── entities
│   │   ├── country
│   │   │   ├── country-delete-dialog
│   │   │   │   ├── country-delete-dialog.component.html
│   │   │   │   ├── country-delete-dialog.component.ts
│   │   │   ├── country-detail
│   │   │   │   ├── country-detail.component.html
│   │   │   │   ├── country-detail.component.ts
│   │   │   ├── country-popup.service.ts
│   │   │   ├── country.module.ts
│   │   │   ├── country.route.ts
│   │   │   └── index.ts
i18n

And of course, i18n also comes to play. So instead of having a flat directory structure:

├── i18n
│   ├── en
│   │   ├── address.json
│   │   ├── contact.json
│   ├── fr

We would also seperate it per micr-service

├── i18n
│   ├── billing     // microserivce
│   ├── invoicing   // microserivce
│   ├── scanning    // microserivce
│   │   ├── en
│   │   │   ├── address.json
│   │   │   ├── contact.json
│   │   ├── fr
Menu

Today the UI shows an "Entity" menu, with all the entities. Again, it should be split into sub-menues (one per microservice) and then, have all the entities of this microservice list.

JHipster Version(s)

4.10.2

gmarziou commented 6 years ago

Using microservice name to package related entities and services makes a lot of sense to me. After all a microservice is supposed to be a bounded context so why not reflecting it in frontend code.

To be consistent with monoliths, I guess we could use a generic package like main to get same folder depth.

While I can see how to apply this to source tree, I'm not sure how to translate this into modules and routes.

Tcharl commented 6 years ago

The microservice package structure looks like a must have for sure. Also, I really like the "package per component" approach too: it would help to modularize the core code: in a project I have, I'm using the 'popup' component as published by jhipster, but had to customize every 'detail' one, so if it were available, I would have made a specific module to generate the detail component the way my customer wanted instead of handwritting everything

gzsombor commented 6 years ago

As the autocomplete fields needs to access the backend, they need the entity-model and the entity-service. Because these fields could be used in every other components, I envision the following structure:

An 'entity/components' module could only depends on the 'entity/core' module. An 'entity/pages' module depends on the 'entity/core', and the component module of the related types. So for example the 'BankAccount' editor dialog could use the 'country-selection' component, without requiring the whole country management.

agoncal commented 6 years ago

@gzsombor if we do that and start to type the model (which I wish JHispter will by getting rid of BaseEntity), you will end up with inter-dependencies again. If an Organisation ---1-> Address ---1-> Country, you end-up with core modules dependency everywhere:

├── billing     // microserivce
├── invoicing   // microserivce
├── scanning    // microserivce
│   ├── entities
│   │   ├── country  
│   │   │   ├── core
│   │   │   │   └── country.model.ts
│   │   │   ├── components
│   │   │   ├── pages
│   │   ├── address  
│   │   │   ├── core
│   │   │   │   └── address.model.ts
│   │   │   ├── components
│   │   │   ├── pages
│   │   ├── organisation  
│   │   │   ├── core
│   │   │   │   └── organisation.model.ts
│   │   │   ├── components
│   │   │   ├── pages

Like the Java back-end, the model should go in the same directory (shared for Angular):

├── billing     // microserivce
├── invoicing   // microserivce
├── scanning    // microserivce
│   ├── entities
│   │   ├── country  
│   │   ├── address  
│   │   ├── organisation  
│   │   ├── shared  
│   │   │   ├── model
│   │   │   │   ├── country.model.ts
│   │   │   │   ├── address.model.ts
│   │   │   │   └── organisation.model.ts
deepu105 commented 6 years ago

Guys I haven't read the proposed structure in detail as I'm on mobile now but please do keep in mind that though we can change folder structure of how microservices entity are organised it may not be nice to change how components are structured we follow the Angular guidelines and do the package by feature so juat make sure that what you propose is inline with Angular guidelines of folder structure. Ill take a detailed look over weekend

On 17 Nov 2017 7:42 am, "Antonio Goncalves" notifications@github.com wrote:

@gzsombor https://github.com/gzsombor if we do that and start to type the model (which I wish JHispter will by getting rid of BaseEntity), you will end up with inter-dependencies again. If an Organisation ---1-> Address ---1-> Country, you end-up with core modules dependency everywhere:

├── billing // microserivce ├── invoicing // microserivce ├── scanning // microserivce │ ├── entities │ │ ├── country │ │ │ ├── core │ │ │ │ └── country.model.ts │ │ │ ├── components │ │ │ ├── pages │ │ ├── address │ │ │ ├── core │ │ │ │ └── address.model.ts │ │ │ ├── components │ │ │ ├── pages │ │ ├── organisation │ │ │ ├── core │ │ │ │ └── organisation.model.ts │ │ │ ├── components │ │ │ ├── pages

Like the Java back-end, the model should go in the same directory (shared for Angular):

├── billing // microserivce ├── invoicing // microserivce ├── scanning // microserivce │ ├── entities │ │ ├── country │ │ ├── address │ │ ├── organisation │ │ ├── shared │ │ │ ├── model │ │ │ │ ├── country.model.ts │ │ │ │ ├── address.model.ts │ │ │ │ └── organisation.model.ts

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/6693#issuecomment-345159067, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlF2JX64Wuwp9Jx3NkmjrWq2yfBYnyks5s3SrigaJpZM4QgZxA .

jdubois commented 6 years ago

Yes, same comment as @deepu105 - we took time to follow John Papa's guidelines, and this shouldn't be broken. Also, this can only be done with a major release, and at the moment we don't have one planned (maybe we'll call React support a major release for marketing purpose, but it won't break anything, so it's not really a major release).

agoncal commented 6 years ago

Well, maybe the John Papa guide has evolved since when the Angular support was added in JHipster, but if you look at the Guide Overall Structural Guidelines, model and services are in the shared directory, to avoid module inter-dependency. As I said, today the JHipster model is not typed (using BaseEntity) that's why we are safe and don't have a big ball of mud.

Model and services should be in shared. Now, when it comes with the new selection components, they should also be in shared

wmarques commented 6 years ago

I agree that the actuel folder structure can be improved. I'm fine with the fact that we should put the models / services into a shared directory, because yes, entities' models and services will probably be used in various components.

Adding a "top level" folder with the microservice name (or a default one for monolith) is also a good idea as it will prevent conflicts in a cleaner way IMO and also structure the project with the folder-by-feature pattern, which is the way to go.

The cons are that this will be a lot of work to do and a big breaking change. I can help on this but I'm already trying to update to NG5 for now, but for me it's a +1.

deepu105 commented 6 years ago

Yes, as @wmarques my main problem is also the amount of work needed. We already have https://github.com/jhipster/generator-jhipster/issues/5833 which according to me is a higher priority than this so unless somebody is willing to take this up I guess its gonna take a while.

agoncal commented 6 years ago

@deepu105 @wmarques maybe this deserves a "task force". Yesterday I was talk to @jdubois about organizing more public hackathons. What if 3/4 developers could physically meet in one of those hackathons and work on such a task for a couple of days ?

deepu105 commented 6 years ago

yes that would really be nice :)

wmarques commented 6 years ago

@agoncal Yep that would be awesome, and I'd be glad to participate of course :)

jdubois commented 6 years ago

It looks like everybody agrees here, but nothing happened :-( Please note we must do also the same thing for React, as the problem is probably the same.

-> do you think this is still doable for v5? Otherwise we will need to close this, as we're not going to keep a ticket opened for more than 1 year.

deepu105 commented 6 years ago

If we are doing this then v5 is the best time otherwise it will never happen. Let me see if i find some time for this after we freeze v4

Thanks & Regards, Deepu

On Mon, Jan 15, 2018 at 8:07 PM, Julien Dubois notifications@github.com wrote:

It looks like everybody agrees here, but nothing happened :-( Please note we must do also the same thing for React, as the problem is probably the same.

-> do you think this is still doable for v5? Otherwise we will need to close this, as we're not going to keep a ticket opened for more than 1 year.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/6693#issuecomment-357765626, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlF4loMDY5ZliogRFWKxdCudwSBvjmks5tK6HigaJpZM4QgZxA .

jdubois commented 6 years ago

yes @deepu105 I totally agree - but changing both Angular + React is a huge task!

deepu105 commented 6 years ago

Since its only about changing directory structure it shouldn't be hard, I have already made sure those are set using variables so should be fine

Thanks & Regards, Deepu

On Mon, Jan 15, 2018 at 8:34 PM, Julien Dubois notifications@github.com wrote:

yes @deepu105 https://github.com/deepu105 I totally agree - but changing both Angular + React is a huge task!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/6693#issuecomment-357770978, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlF8Jx3L5QNvmS0LiiOV7m-eM0hTAYks5tK6hPgaJpZM4QgZxA .

deepu105 commented 6 years ago

I'll assign this to myself and will implement something midway based on what @agoncal proposed and what I had in mind. I do have some idea already. For React its easier as we dont have similar interdependency issues due to the async nature of redux/react

agoncal commented 6 years ago

@deepu105 I'll be more than happy to test it. We already have a complex microservices architecture and keep the JDL up to date. So for me it will just be a matter of taking your PR and regenerate the code.

I was also hoping to see https://github.com/jhipster/generator-jhipster/pull/6618 as it is slightly related. I'm going to ping @gzsombor

agoncal commented 6 years ago

@deepu105 Hey, do you think this could be done in JHipster 5? A force task would be great ;o)

deepu105 commented 6 years ago

@agoncal @jhipster/developers to be less disruptive to people who upgrade I'm considering the below, let me know if you are ok/not ok with it

  1. For microservices, the entity folder structure will be (since services are only imported in components they don't result in cyclic dep issue if user needs to aggregate services s/he should do it in the component or write a new one in shared)
    |_ entities
    |_ microserviceName
    |_ entityName
      |_ *.{component,module,service,route}.{ts,html}
    |_ shared
      |_ *.model.ts
  2. For monoliths
    |_ entities
    |_ entityName
      |_ *.{component,module,service,route}.{ts,html}
    |_ shared
      |_ *.model.ts
  3. Remove BaseEntity and use strongly typed interfaces only
jdubois commented 6 years ago

Yes that looks good to me @deepu105 - that's basically the same as what we have for APIs, so it's easy to understand.

agoncal commented 6 years ago

@deepu105 I strongly recommend having services also in the shared. If #6618 is integrated, that means auto-complete fields will need services:

|_ entities
  |_ microserviceName
    |_ entityName
      |_ *.{component,module,route}.{ts,html}
    |_ shared
      |_ model
        |_ *.model.ts
      |_ service
        |_ *.service.ts
      |_ selection
        |_ *.{selection}.{ts,html}

That would mean selection components, and entities components will both need the service.

WDYT ?

gmarziou commented 6 years ago

I wonder also if we could have a client source code structure that would make possible having multiple single page applications. This need is often raised by people wanting to keep our generated UI as a backoffice while adding their own end user UI.

deepu105 commented 6 years ago

@agoncal the reason I didn't include services is that you need the services to be registered as a provider in a module and hence this would become more difficult. Let me look at the autocomplete PR and see if I can find a solution. If possible I'll try to move them there.

IMO we shouldn't change too drastically as it will be extremely painful to upgrade and might even be confusing to people.

agoncal commented 6 years ago

@gmarziou That's exactly our need. ATM we do plenty of nasty hacks to "kind of get it right". One of them is that we rename the /api of JHipster to /internal, and then we build our own API on /api. This way we distinguish which APIs is JHipster's and which are our own. For the UI components, we use subdirectories and subcomponents and subservices... but it's ugly.

gmarziou commented 6 years ago

I agree that this should be easier like setting few properties in .yo-rc.json that would be used for generating code similar to jhiPrefix.

gzsombor commented 6 years ago

In my projects, we ended up creating

Yes, it's only useful, if the project grows to more than a couple of entities, and this is hard to automatize :(

gmarziou commented 6 years ago

About circular dependencies, my approach is two fold:

@deepu105 and @sendilkumarn, it seems to me that in React you are using Redux, is it for state management? If so, shouldn't we apply same pattern in Angular?

agoncal commented 6 years ago

@gzsombor @gmarziou Reading what you do and what we do, it looks like there are different patterns in using JHipster. Wouldn't it be great to document these patterns and adapt the code to these patterns? The patterns I see:

We see a great value in "side by side". That's why we would like to have a few changes in the Angular structure.

gmarziou commented 6 years ago

It's a great idea to name these patterns, we can then document them and provide tools to support them when possible. I'm currently doing some work on this and will share soon. My only concern is that to support them better we could need some breaking changes that we can do only for JHipster 5.

agoncal commented 6 years ago

@gmarziou @deepu105 is also concerned about too many breaking changes. But isn't that what a major is there for ;o)

gmarziou commented 6 years ago

:) I'm more concerned by complexity, v4 has added a lot of features ! This reminded me this presentation when he talks about Django and then JS frameworks. Sorry for spamming

deepu105 commented 6 years ago

@agoncal I'm not too concerned about breaking changes in the major release, I'm more concerned about doing drastic changes for use cases which may not be very common. I also agree with @gmarziou's comment about complexity. I don't want to end up doing something very complex for most people.

I can see that slowly the suggestions are mimicking package by type, which I personally don't like and is definitely not what is suggested as best practice by front-end community at least. So i'll try to address as much of the concerns raised here but I can't promise to address all of them, there needs to be some compromise in terms of complexity, maintenance etc.

I'll do this as a PR so that everyone can have a look at it and raise concerns if any. I can also try to provide some tricks for power users like @agoncal to achieve some customizations that he is looking for

@gmarziou Redux is nice with React but I'm not a big fan of Redux with Angular and also its a big rewrite which i'm not willing to undertake.

agoncal commented 6 years ago

@deepu105 I understand your concerns. More than happy to check your PR and give you some feedback

gmarziou commented 6 years ago

I can see that slowly the suggestions are mimicking package by type

Probably because this is what we have also server side and it is difficult to do otherwise without having business requirements to help for taking decisions.