Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding app-shell breaks down lazy loading on server-side #9202

Closed
krivochenko opened this issue Jan 14, 2018 · 12 comments · Fixed by #12587
Closed

Adding app-shell breaks down lazy loading on server-side #9202

krivochenko opened this issue Jan 14, 2018 · 12 comments · Fixed by #12587
Labels
area: devkit/build-angular freq3: high P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity5: regression type: bug/fix
Milestone

Comments

@krivochenko
Copy link

Versions

Angular CLI: 1.6.4
Node: 8.9.1
OS: win32 x64
Angular: 5.2.0
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, platform-server, router
... service-worker

@angular/cli: 1.6.4
@angular-devkit/build-optimizer: 0.0.38
@angular-devkit/core: 0.0.25
@angular-devkit/schematics: 0.0.48
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.9.4
@schematics/angular: 0.1.13
@schematics/schematics: 0.0.13
typescript: 2.5.3
webpack: 3.10.0

Repro steps

  • git clone git@github.com:krivochenko/lazy.git
  • npm install
  • build:ssr
  • serve:ssr - on this step everything works fine. Including server-side rendering with lazy loading. There isn't errors in console of server.
  • ng generate app-shell --universal-app universal
  • build:ssr
  • serve:ssr

Observed behavior

Server-side rendering doesn't work (in source code of page I see only router-outlet). In console of server I get error:

Cannot find module 'app/pages/contacts/contacts.module.ngfactory'

Desired behavior

I expect server-side rendering work without errors.

Mention any other details that might be useful (optional)

I created simple repo for reproduction of issue by following steps:

  • ng new lazy --service-worker --routing
  • adding HomeComponent and ContactsComponent for checking lazy loading
  • adding universal followed with guide

Thanks for help

@krivochenko
Copy link
Author

As I understand, using App Shell have a meaning only if I don't use server-side rendering. If I use Universal-app for server-side rendering, my web-server returns not blank page, so using App Shell is redundant.

But I need some kind of App Shell for PWA, so I have to provide some URL with App Shell by myself without using ng generate app-shell ...

Please correct me, if I missed something.

@hansl hansl added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent feature: universal freq3: high severity5: regression labels Jan 22, 2018
@hansl hansl unassigned Brocco Feb 6, 2018
@FrozenPandaz
Copy link
Contributor

Hi,

I have investigated this issue to a degree. I also cannot find any way of using app-shells with Universal...

Sorry i wrote a novel, feel free to skip down to the solutions

Conflict:

  • Prerendering the app shell is done with the app server module
  • The ModuleMapLoaderModule requires a module map, without it the app shell wont render
  • It seems extraProviders during bootstrap are the bottom layer of providers
    • Providing a dummy module map or the app shell rendering will fix that but break lazy loaded routes on the server as mentioned by @krivochenko

Solutions:

  • Yes, you can have a normal server app with an app shell route and have the ngsw-config point to the route with the app shell instead of index.html
    • When someone installs a version of the PWA, it will go to the server rendered route and you will have your app shell
    • You can do this today but it's a little wasteful to have the server generate it at request time
    • It should be possible to prerender the app shell at build time AND server render
  • You can write your own post process script to render the app shell but this is what the CLI intends to do for you
  • These are possible solutions for the CLI
    • When rendering the app shell, it provides a dummy module map.
      • This should be the simplest/ easiest solution but it adds a (Angular team controlled) dependency to the CLI
    • The ModuleMapNgFactoryLoader is modified to have the module map be optional so that DI doesn't break
      • I had some trouble with this having a null module map even though one was provide in extraproviders in the server executable
      • This also isn't exactly kosher because the module map is pretty much require for the provider to work

@krivochenko Your solution should work, if you need an example, let me know

@hansl @Brocco Should we pursue providing a dummy moule map here?

It would look like

    const html = await renderModuleFactory(AppServerModuleNgFactory, {
        url: options.url,
        document: index,
        extraProviders: [
            provideModuleMap({})
        ]
    });

@playground
Copy link

@Brocco you mentioned that app shell feature in CLI is not documented when we spoke in ng-atl. Can you tell me what is the process to get that documented? Thanks.

@evanjmg
Copy link

evanjmg commented Sep 5, 2018

Any update on this?

@alan-agius4
Copy link
Collaborator

alan-agius4 commented Sep 7, 2018

I looked into this and the problem is that when using the RouterModule.forRoot() in app.server.module.ts NgModuleFactoryLoader is not being provided as ModuleMapNgFactoryLoader from @nguniversal/module-map-ngfactory-loader but rather NgModuleFactoryLoader from @angular/core as the RouterModule is bootstrapped in prior to ModuleMapLoaderModule

To fix this one need to add the following in AppServerModule so that the provider is updated across the board;

  providers: [{
    provide: NgModuleFactoryLoader,
    useClass: ModuleMapNgFactoryLoader
  }],

That said, this only works with the latest packages of the dependencies.

Created an example repo here: https://github.com/alan-agius4/universal-lazy-loading-app-shell

//cc @vikerman

@vikerman
Copy link
Contributor

@alan-agius4 - The provider should be used at the platform providers level and not the ServerAppModule

https://github.com/angular/universal-starter/blob/master/server.ts#L28

@alan-agius4
Copy link
Collaborator

@vikerman that does work as well. Thanks for your input

alan-agius4 added a commit to alan-agius4/universal-starter that referenced this issue Oct 15, 2018
At the moment, there is no clear explaination how to make an app-shell work togather with SSR and lazy loading.

See for more context angular/angular-cli#9202
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Oct 16, 2018
CaerusKaru pushed a commit to angular/universal-starter that referenced this issue Oct 18, 2018
… ssr (#646)

At the moment, there is no clear explaination how to make an app-shell work togather with SSR and lazy loading.

See for more context angular/angular-cli#9202
kyliau pushed a commit that referenced this issue Oct 23, 2018
kyliau pushed a commit that referenced this issue Oct 23, 2018
@csbenjamin
Copy link

@alan-agius4 your solution does not work for me. I keep getting the error: Cannot find module 'app/pages/contacts/contacts.module.ngfactory'

@csbenjamin
Copy link

I solve my problem doing the following:

ng run my-app:app-shell:production
sed -i -e "s/RouterModule.forRoot/\\/\\/ RouterModule.forRoot/g" path/to/app.server.module.ts
ng run my-app:server
sed -i -e "s/\\/\\/ RouterModule.forRoot/RouterModule.forRoot/g" path/to/app.server.module.ts

@alan-agius4
Copy link
Collaborator

Hi @csbenjamin, please note that this issue is closed.

If this issue is still happening even with changes stated in the documentation, please open a new issue with a minimal reproduction and a step by step guide. A good way to make a minimal repro is to create a new app via ng new repro-app and adding the minimum possible code to show the problem. Then you can push this repository to github and link it here.

This might be related to your directory structure so its really important to get an accurate repro to diagnose this.

@csbenjamin
Copy link

@alan-agius4 Thanks for the reply. Here is the new issue
#12921
There I put a repo as you ask
https://github.com/csbenjamin/app-shell-plus-univeral-plus-lazy-routing

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: devkit/build-angular freq3: high P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity5: regression type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants