angular-cli icon indicating copy to clipboard operation
angular-cli copied to clipboard

App Shell not generated when wildcard route (not found page) is present in app

Open bellizio opened this issue 8 years ago • 19 comments

Versions

Angular CLI: 1.6.1
Node: 8.9.0
OS: darwin x64
Angular: 5.1.1
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, platform-server, router

@angular/cli: 1.6.1
@angular-devkit/build-optimizer: 0.0.36
@angular-devkit/core: 0.0.22
@angular-devkit/schematics: 0.0.42
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.9.1
@schematics/angular: 0.1.11
@schematics/schematics: 0.0.11
typescript: 2.4.2
webpack: 3.10.0

Repro steps

  • Follow the steps to add an app shell to angular universal app as described here
  • Create a 'not-found' component with the following angular-cli command: ng generate component not-found --skip-import
  • Import and add the NotFoundComponent to app-routing.module.ts:
import { NgModule } from '@angular/core';
import { Routes, RouterModule } from '@angular/router';
import { NotFoundComponent } from './not-found/not-found.component';

const routes: Routes = [
  { path: '**', component: NotFoundComponent }
];

@NgModule({
  imports: [RouterModule.forRoot(routes)],
  exports: [RouterModule]
})
export class AppRoutingModule { }
  • Import and add NotFoundComponent to declarations array in app.module.ts
  • Run ng build --prod
  • Inspect dist/index.html

Observed behavior

dist/index.html includes the <app-not-found> tag for the NotFoundComponent. See the snippet below:

<router-outlet _ngcontent-c0=""></router-outlet><app-not-found _nghost-c1=""><p _ngcontent-c1="">
  not-found works!
</p>
</app-not-found>

Desired behavior

dist/index.html should include the <app-app-shell> tag for the AppShellComponent. See the snippet below:

<router-outlet _ngcontent-c0=""></router-outlet><app-app-shell _nghost-c1=""><p _ngcontent-c1="">
  app-shell works!
</p>
</app-app-shell>

Mention any other details that might be useful (optional)

Based on the documentation I could find about the App Shell feature introduced in Angular 5.1 (see here & here), I could not find anything detailing how the app shell should work when a wildcard route (aka 'not found' page) is present in an app's routing config.

How should an app with a wildcard route be configured to support an app-shell and its accompanying route?

bellizio avatar Dec 18 '17 21:12 bellizio

I'm experiment something related. In my case I've added the wildcard route after have created the app-shell.

{ path: '', redirectTo: '/home', pathMatch: 'full'}, { path: '**', component: NotFoundComponent }

Then the app-shell component is not being show before redirecting to the home-page anymore, instead the NotFoundComponent is being showed before home.

cvgaviao avatar Jan 10 '18 14:01 cvgaviao

I've also run into the same thing. Its as if the server build doesn't ignore the routes added in App module when you import it into AppServerModule.

I thought maybe adding a wild card in the server routes that redirected to the app-shell path would maybe help, but alas. It did not.

d-nation avatar Jan 24 '18 17:01 d-nation

This might be a problem with @angular/platform-server since all the cli does is call renderModuleFactory https://github.com/angular/angular-cli/blob/cfc58a02660d6159db8da20ceb1fbe18cfee2e45/packages/%40angular/cli/tasks/render-universal.ts#L30

Is there a way to unregister a route?

d-nation avatar Jan 24 '18 23:01 d-nation

The problem lies in that both AppModule and AppShellModule register routes, and that routes defined in the AppShellModule somehow end up registered last, independent of import order.

The complete route config ends up looking like this:

[
  { ...otherRoutes },
  { path: '**', redirectTo: '/' },
  { path: 'shell', component: [] }
];

Because of this the shell route will never be matched due to the wildcard route that is registered before it.

Edit: the possible solution (changing import order) I posted earlier does not work unfortunately.

Manduro avatar Jan 30 '18 15:01 Manduro

@Manduro I just tried your solution, but the issue still exists.

bellizio avatar Jan 30 '18 16:01 bellizio

same here - mine is already configured that way and doesn't work:


const routes: Routes = [ { path: 'shell', component: AppShellComponent }];

@NgModule({
  imports: [
    RouterModule.forRoot(routes),
    ServerModule,
    AppModule
  ],
  bootstrap: [AppComponent],
  declarations: [AppShellComponent],
})
export class AppServerModule {}

abritabroad avatar Jan 30 '18 16:01 abritabroad

That didn't help me either. For some context, here's my browser routes:

const routes: Routes = [
  {
    path: '',
    component: LayoutComponent,
    children: [
      {
        path: '',
        component: RouteAComponent,
        canActivate: [ AuthGuard ]
      },
      {
        path: '**',
        component: PageNotFoundComponent
      }
    ]
  }
];

So I was thinking that perhaps the top level path: '' was catching the 'app-shell' route so i changed my server routes to const routes: Routes = [ { path: '', children: [ {path: 'app-shell', component: AppShellComponent } ] } ];

That did not work regardless of whether AppModule was imported before or after RouterModule in AppServerModule

d-nation avatar Jan 30 '18 16:01 d-nation

Sorry for what I posted earlier, I tested incorrectly and thought it was working. This time I investigated further and found a way to override the routes when rendering the app shell. Use the following workaround in your app.server.module.ts:

const routes: Routes = [{ path: 'shell', component: AppShellComponent }];

@NgModule({
  imports: [
    AppModule,
    ServerModule,
    RouterModule.forRoot(routes)
  ],
  bootstrap: [AppComponent],
  declarations: [AppShellComponent]
})
export class AppServerModule {
  // The important part:
  constructor(private router: Router) {
    this.router.resetConfig(routes);
  }
}

This removes all routes that are imported by your regular AppModule, and replaces it with the app shell route, so the wildcard route is not blocking it from loading.

Manduro avatar Jan 31 '18 10:01 Manduro

@Manduro I just tried your override solution, and while the app-shell component now appears in index.html after running ng-build --prod, the not-found component is rendered on the home page when running ng-serve --prod.

bellizio avatar Jan 31 '18 13:01 bellizio

@Manduro 's solution works for me as well. @Brocco should this be added to the blueprint? Or the documentation? I'd be happy to submit a PR for either depending on the "route" the team wants to take (See what I did there?)

d-nation avatar Feb 01 '18 19:02 d-nation

@bellizio This is really late, but that is expected behavior. App-shell is not supposed to work for ng-serve even with --prod. To test it, you have to do a build, and then use something like http-server ./dist (assuming you have http-server installed)

literalpie avatar Apr 25 '18 17:04 literalpie

@literalpie to your point, I think you are correct in that the ng serve --prod command can not be used to properly test app-shell in the browser. I just tried doing so and the app-shell is nowhere to be found.

You have to generate a build with the ng build --prod command, and use a static server like http-server to serve it.

After experimenting further, here is what ultimately worked for me:

app-routing.module.ts

import { NgModule } from '@angular/core';
import { Routes, RouterModule } from '@angular/router';
import { NotFoundComponent } from './not-found/not-found.component';

const routes: Routes = [
  { path: '', redirectTo: '', pathMatch: 'full' },
  { path: '**', component: NotFoundComponent }
];

@NgModule({
  imports: [RouterModule.forRoot(routes)],
  exports: [RouterModule]
})
export class AppRoutingModule {}

app.server.module.ts

const routes: Routes = [
  { path: 'app-shell-path', component: AppShellComponent }
];

@NgModule({
  imports: [AppModule, ServerModule, RouterModule.forRoot(routes)],
  bootstrap: [AppComponent],
  declarations: [AppShellComponent]
})
export class AppServerModule {
  constructor(private router: Router) {
    this.router.resetConfig(routes);
  }
}

Note the addition of { path: '', redirectTo: '', pathMatch: 'full' } in the routes. Although this is a simple solution and not something you would likely find in a real-world app, it is necessary for this example. Without it, Angular will not see the root url path (e.g. localhost:8080) as a valid route. And in that case, the not-found component will be rendered at the root url path (after app-shell first renders).

Additionally, this.router.resetConfig(routes); must be called in AppServerModule as @Manduro pointed out. Without it, app-shell is not added to index.html after ng build --prod is run and not-found is added instead.

In order to test app-shell in this example, it is best to use network throttling in Chrome or Firefox (I tested with 2G and 3G).

All this is to say that I think this should still remain as an open bug because of the requirement to call this.router.resetConfig(routes);, which is not documented. Perhaps as a fix for this, Angular could do this internally so that devs to not manually have to. Thoughts?

bellizio avatar Apr 25 '18 19:04 bellizio

Heya, is this still a problem in the latest CLI versions?

filipesilva avatar Oct 08 '19 13:10 filipesilva

it's still an issue in the last CLI version. Same thing the app shell generate the page not found instead of the shell component. I still need to add the following code in the AppServerModule export class.

constructor(private router: Router) {
    this.router.resetConfig(routes);
}

nsabourin avatar Jan 20 '20 14:01 nsabourin

my solution would be to split the app.module into two, one app.module that has everything except route, and a app.browser.module that imports it and adds the route, and of course the rest of the client app would use the app.browser.module... I did this once with universal to cut out the AnimationModule

But today, I just added the "shell" route to the routing module, just before the catch all, I thought I do not wish to lose my main components, so why not? It is just one extra component to the client side.

ayyash avatar Jan 20 '20 18:01 ayyash

Yes, there's options to get around this issue. But, I only wanted to notice that it is still an issue in the last version of Angular CLI. @filipesilva @ayyash

nsabourin avatar Jan 20 '20 22:01 nsabourin

I did the this.router.resetConfig(routes); solution but instead of app-shell now nothing is presented in <app-root>. And CLI says that app-shell generated successfully.

mahdizarei0614 avatar Nov 16 '21 12:11 mahdizarei0614

This problem is still existing in angular 15. The router reset works to generate the app-shell. But it breaks the SSR render to render the other routes.

To fix this, you can import the app routes and pass it along as a new Routes object. Make sure the app-shell routes come before the app routes.

  // app.server.module.ts
  constructor(private router: Router) {
    this.router.resetConfig([...serverRoutes, ...appRoutes]);
  }

marcowuethrich avatar Dec 15 '22 15:12 marcowuethrich

How shall we configure this for a standalone project? @Manduro thx ❤

app.config.server.ts file:

import { mergeApplicationConfig, ApplicationConfig } from '@angular/core';
import { provideServerRendering } from '@angular/platform-server';
import { appConfig } from './app.config';
import { ROUTES } from '@angular/router';
import { AppShellComponent } from './app-shell/app-shell.component';

const serverConfig: ApplicationConfig = {
  providers: [
    provideServerRendering(),
    {
      provide: ROUTES,
      multi: true,
      useValue: [
        {
          path: 'shell',
          component: AppShellComponent,
        },
      ],
    },
  ],
};

export const config = mergeApplicationConfig(appConfig, serverConfig);

I also experienced that if I use the APP_INITIALIZER and try to redirect to an error page (based on our logic), App-shell navigates to that error page and embeds that page into our HTML instead of embedding the app-shell page itself! How this can be resolved?!

app.config.ts file:

import { APP_INITIALIZER, ApplicationConfig, isDevMode } from '@angular/core';
import {
  Router,
  provideRouter,
  withEnabledBlockingInitialNavigation,
} from '@angular/router';
import { appRoutes } from './app.routes';

function initAppFactoryConfig(router: Router) {
  return () => new Promise((resolve, reject) => {

    // We resolve the promise whatsoever! Because we want the app to complete 
    // its initialization... BUT based on our logic (e.g., if the file we're 
    // trying to load at initialization, couldn't get loaded), we may like to 
    // redirect to one of our app's pages and then resolve... In this case 
    // app-shell embeds the error page into the HTML instead!
    router.navigate(['/error-loading']);
    resolve(true);
  });
}

export const appConfig: ApplicationConfig = {
  providers: [
    provideRouter(appRoutes, withEnabledBlockingInitialNavigation()),
    {
      provide: APP_INITIALIZER,
      useFactory: initAppFactoryConfig,
      deps: [Router],
      multi: true,
    },
  ],
};

imalitavakoli avatar Sep 25 '23 18:09 imalitavakoli

Thanks for reporting this issue. Luckily, it has already been fixed in one of the recent releases. Please update to the most recent version to resolve the problem.

If the problem persists in your application after upgrading, please open a new issue, provide a simple repository reproducing the problem, and describe the difference between the expected and current behavior. You can use ng new repro-app to create a new project where you reproduce the problem.

alan-agius4 avatar May 07 '24 12:05 alan-agius4

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.