search

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.27k stars 4k forks source link

Angular 2 TODO for next #4794

Closed deepu105 closed 6 years ago

deepu105 commented 7 years ago

This ticket is to track TODO items after the angular2 release as per #4549

Items with - to verify first needs to be checked/tested first to see if already implemented

Angular

webpack

General

VictorADS commented 7 years ago

@deepu105 I got HMR working but there is a problem. When we change angular 2 files like html or ts the whole angular 2 module is refreshed causing a blink in the screen. I thought it was i18n loading but in reality I'm pretty sure it's ui-router that causes it because I tried angular/router and everything works fine.

If you want to test iit I made a repo : https://github.com/VictorADS/ngHmr

The branch master contains the version with angular/router and the branch hmr-uirouter contains the version with ui-router. To test you need to run the server and run npm run webpack:hmr

deepu105 commented 7 years ago

ok lets do it after our release when we move to angular router

gmarziou commented 7 years ago

@VictorADS maybe this could be avoided by importing all deps in vendor.ts so that main bundle contrains only jhipster code

seawenzhu commented 7 years ago

Hi @deepu105 What is plan when you move to angular router?

VictorADS commented 7 years ago

@gmarziou I tried but the blink is still there. Besides this little visual issue the HMR works great and it reloads the page really fast. So can I make a PR ?

deepu105 commented 7 years ago

Yes we can live with the blinks I guess

Thanks & Regards, Deepu

On Mon, Jan 9, 2017 at 12:06 PM, VictorADS notifications@github.com wrote:

@gmarziou https://github.com/gmarziou I tried but the blink is still there. Besides this little visual issue the HMR works great and it reloads the page really fast. So can I make a PR ?

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

deepu105 commented 7 years ago

Could you also try to do some lazy chunks?

Thanks & Regards, Deepu

On Mon, Jan 9, 2017 at 12:07 PM, Deepu K Sasidharan d4udts@gmail.com wrote:

Yes we can live with the blinks I guess

Thanks & Regards, Deepu

On Mon, Jan 9, 2017 at 12:06 PM, VictorADS notifications@github.com wrote:

@gmarziou https://github.com/gmarziou I tried but the blink is still there. Besides this little visual issue the HMR works great and it reloads the page really fast. So can I make a PR ?

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

VictorADS commented 7 years ago

@deepu105 What I understood about lazy chunks loading is that we load the modules when we try to access a specific state. Wouldn't it be better to migrate to angular-router then add lazy loading ?

deepu105 commented 7 years ago

Yes you are right. I was talking more about webpack chunks sorry for the confusion

Thanks & Regards, Deepu

On Mon, Jan 9, 2017 at 5:59 PM, VictorADS notifications@github.com wrote:

@deepu105 https://github.com/deepu105 What I understood about lazy chunks loading is that we load the modules when we try to access a specific state. Wouldn't it be better to migrate to angular-router then add lazy loading ?

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

VictorADS commented 7 years ago

What part of the app do you want to lazy load ? I was thinking about admin / account / entities module but that would be done within the routing files.

deepu105 commented 7 years ago

​I'm not very sure. I think now the majority of the time is spent in compiling stuff. can we try to make sure that the vendor files are not compiled during file changes in app first.​

Thanks & Regards, Deepu

On Tue, Jan 10, 2017 at 2:50 PM, VictorADS notifications@github.com wrote:

What part of the app do you want to lazy load ? I was thinking about admin / account / entities module but that would be done within the routing files.

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

VictorADS commented 7 years ago

I'm looking into DLL webpack plugin which might be what you want

deepu105 commented 7 years ago

Yes I missed it

Thanks & regards, Deepu

On 18 Jan 2017 1:28 a.m., "Gaël Marziou" notifications@github.com wrote:

Maybe an additional task:

  • add some needles for modules, for instance in vendor.ts.

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

gmarziou commented 7 years ago

Regarding lazy loading sub modules, I have few comments:

gmarziou commented 7 years ago

What about adding analytics? https://github.com/angulartics/angulartics2/

deepu105 commented 7 years ago

I think we already have GA setup in index.html and I think it should work same as we had earlier

Thanks & Regards, Deepu

On Tue, Jan 31, 2017 at 2:20 PM, Gaël Marziou notifications@github.com wrote:

What about adding analytics? https://github.com/angulartics/angulartics2/

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

gmarziou commented 7 years ago

I think GA will only catch access to '/', angulartics2 knows about the router and can report all route changes to GA.

deepu105 commented 7 years ago

Isnt this something better suited for a Module?

Thanks & Regards, Deepu

On Tue, Jan 31, 2017 at 3:15 PM, Gaël Marziou notifications@github.com wrote:

I think GA will only catch access to '/', angulartics2 knows about the router and can report all route changes to GA.

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

gmarziou commented 7 years ago

OK

gmarziou commented 7 years ago

In fact, it seems really simple to do by subscribing to route changes: https://blog.thecodecampus.de/angular-2-google-analytics-google-tag-manager/

deepu105 commented 7 years ago

@jhipster/developers guys is anybody working on the TODO items here?

jdubois commented 7 years ago

@deepu105 no I don't think anybody does, and there is a lot of stuff here. AOT is specifically annoying to me, as we really need this for production.

jdubois commented 7 years ago

@deepu105 what should we do about this list? It looks to me like we are stucked...

deepu105 commented 7 years ago

We need help on this. More hands and feedback

On 13 Apr 2017 11:15 am, "Julien Dubois" notifications@github.com wrote:

@deepu105 https://github.com/deepu105 what should we do about this list? It looks to me like we are stucked...

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

tezcane commented 7 years ago

I can do lazy loading of modules.

deepu105 commented 7 years ago

Yes please. Please note that for entities its trickier when they have relationships with other entities

On Fri, 14 Apr 2017, 9:04 am tezcane, notifications@github.com wrote:

I can do lazy loading of modules.

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

gmarziou commented 7 years ago

Indeed lazy loading entity modules seems too complex, what about just lazy-loading the user management part?

sendilkumarn commented 7 years ago

Yeah, that would be a great start.

deepu105 commented 7 years ago

We could lazy load entire admin modules actually

On Sat, 15 Apr 2017, 6:13 am Sendil Kumar N, notifications@github.com wrote:

Yeah, that would be a great start.

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

jdubois commented 7 years ago

Yes we should, as it will soon become optional !

tezcane commented 7 years ago

I will not be able to get to this for 3 weeks, is that OK?

tezcane commented 7 years ago

I just lazy loaded my entire Ionic 3 app (~20 components). It consisted of adding another @IonicPage() decorator over the @Component() then making a .module.ts file:

import {NgModule       } from "@angular/core";
import {IonicPageModule} from "ionic-angular";
import {TranslateModule} from "@ngx-translate/core";

import {SuperTabsModule} from 'ionic2-super-tabs';

import {PageTerms} from "./terms";

@NgModule({
  declarations: [PageTerms],
  imports:      [TranslateModule.forChild(),
                 SuperTabsModule,
                 IonicPageModule.forChild(PageTerms)],
  exports:      [PageTerms]
})
export class ModulePageTerms {}

I'd imagine the NG4 updates to do lazy loading is similar, JHipster code is already full of .module.ts files.

Alwan commented 7 years ago

I added lazy loading you can check the repo
https://github.com/Alwan/generator-jhipster/tree/lazy-loading and sample project https://github.com/Alwan/jhipster-lazy-loading-sample

the changes are:

need more testing. two things not working 1- the translation not showing in the lazy loaded modules when you refresh the page. 2- the urls for popup dialog not working properly.

gmarziou commented 7 years ago

What are the benefits of lazy loading entities?

Alwan commented 7 years ago

You don't need to load all entities when you browsing the home page.

In production my application start up time improved and the bundle size went from 600k+ to 350k.

gmarziou commented 7 years ago

In fact, I imagine that some developers may keep generated entity views for managing data as an admin user but most of them may probably write custom views for normal users while using generated data services.

Please excuse my ignorance but what is really lazy-loaded for an entity route? If the full entity module (component, template, service, routes) is lazy-loaded, does it mean that user will encounter an error if I try to use an entity service from another view that is not already loaded?

Alwan commented 7 years ago

as I understand it the whole module will be in a separated file. You can check it in Chrome dev tools.

Yes if you try to use an entity service you have to add it to your module providers. for example If you want to use an entity in home page go to HomeMudule providers and add entityService.

gmarziou commented 7 years ago

OK, so it looks great.

deepu105 commented 7 years ago

@Alwancan you do a PR?

jdubois commented 7 years ago
deepu105 commented 7 years ago

@jdubois ill check those over the weekend.

  1. Lazy loading is good but not a necessity so that can even wait if we are unable to merge it soon.
  2. Btw why do you want them in separate folders? If we clean the www directory before prod build it works fine. Im doing the same with my prod app and works fine. It also simplifies packaging as we dont have to handle any profile specific stuff. Can you let me know what is the issue you foresee? 3.ill check that probably something minor in config
jdubois commented 7 years ago

Le 24 mai 2017 8:09 AM, "Deepu K Sasidharan" notifications@github.com a écrit :

@jdubois https://github.com/jdubois ill check those over the weekend.

  1. Lazy loading is good but not a necessity so that can even wait if we are unable to merge it soon.
  2. Btw why do you want them in separate folders? If we clean the www directory before prod build it works fine. Im doing the same with my prod app and works fine. It also simplifies packaging as we dont have to handle any profile specific stuff. Can you let me know what is the issue you foresee? 3.ill check that probably something minor in config

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

deepu105 commented 7 years ago

@jhipster/developers I know some of you are testing angular 2+ to get it out of Beta, could you please update the first entry in this thread with issue you find and also tick of items in the list that works.

@jdubois I saw in another thread that you mentioned you found issues could you please put them up here so that I can try to close items. I have some time today and on weekend

deepu105 commented 7 years ago

@jhipster/developers Also plz put your name beside any issue if you are working on it so that we dont do double work on same items

jdubois commented 7 years ago

@deepu105 I added 3 items from my latest tests. I don't have my laptop until Sunday but I'm of course available for help/discussions (anything I can do with m'y mobile phone)

tezcane commented 7 years ago

Just following up here, I did not implement this because Alwan looks to have taken it over.

gmarziou commented 7 years ago

I took this one:

But I think we should not do it because it raises many problems about how to determine whether a URL belongs to server namespace or to client namespace. I implemented it as a servlet filter based on request method and some regexp (see below). Maybe it could be improved if there's a way to get the list or mappings from Spring MVC.

Anyway this implementation looks brittle with our current URI design, conflicts between server and client routes could happen quite easily with customizations of generated code and could be confusing. Things could be even worse for users who setup a context path or who get one by deploying to a server like WebLogic, etc...

We could avoid many of these traps by using a prefix like '/app/' for all client routes but is it a better solution than a '#' ?

AngularRouteFilter .java
```java package com.mycompany.myapp.web; import org.springframework.web.filter.OncePerRequestFilter; import javax.servlet.FilterChain; import javax.servlet.RequestDispatcher; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.regex.Pattern; public class AngularRouteFilter extends OncePerRequestFilter { // add the values you want to redirect for private static final Pattern PATTERN = Pattern.compile("^/((api|swagger-ui|management|swagger-resources)/|favicon\\.ico|v2/api-docs).*"); @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { if (isServerRoute(request)) { filterChain.doFilter(request, response); } else { RequestDispatcher rd = request.getRequestDispatcher("/"); rd.forward(request, response); } } protected static boolean isServerRoute(HttpServletRequest request) { if (request.getMethod().equals("GET")) { String uri = request.getRequestURI(); return PATTERN.matcher(uri).matches(); } return true; } } ```
deepu105 commented 7 years ago

@gmarziou yes its tricky. Anyway its not a must for making angular out of BETA so lets take it out from the list for now. We didnt do it for ng1 as well anyway

deepu105 commented 7 years ago

I have descoped some items as they are not required to get Angular out of BETA

gmarziou commented 7 years ago

Yes, I'll try to write it as a tip with some words of warning.