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

Library shim override strategy in gotchas.md doesn't seem to work

Open benelliott opened this issue 2 years ago • 12 comments

🐞 Bug report

What modules are related to this issue?

  • [ ] aspnetcore-engine
  • [ ] builders
  • [x] common
  • [ ] express-engine
  • [ ] hapi-engine

Is this a regression?

Probably

Description

The "library shim" strategy described in https://github.com/angular/universal/blob/master/docs/gotchas.md for fixing "window is not defined" errors doesn't seem to work for Angular 13. This has caused me to get stuck as I cannot find a way around these errors for my own app (proprietary Universal-incompatible library used in a proprietary app).

This is related to angular/universal#1675 but specifically focuses on the incorrect documentation.

🔬 Minimal Reproduction

Going by the example snippets in gotchas.md, the compiler will throw an error if LibraryComponent cannot be resolved via the imports from AppModule alone.

To create a reproduction, assume for the sake of argument that @angular/material's mat-checkbox component is not Universal-compatible - it becomes our library-component.

Here is a reproduction that tries to be as close as possible to the instructions in gotchas.md. The steps i followed to create it were:

  • Create a new project via ng new
  • ng add @angular/material
  • ng add @nguniversal/express-engine
  • ng g c homepage --skip-import
  • ng g c example --skip-import
  • Edit ExampleComponent's template to be <mat-checkbox></mat-checkbox>
  • Edit HomepageComponent's template to be <app-example></app-example>
  • Add ExampleComponent and HomepageComponent to declarations of AppModule (the shared module), but don't import MatCheckboxModule
  • Create browser-app.module.ts with imports of AppModule and MatCheckboxModule (i.e. providing the in-browser implementation for the Universal-incompatible component in the browser-specific module)
  • Configure routes as [{path: '', component: HomepageComponent}]
  • Run ng serve or ng build to compile the app in browser mode

This results in a compilation error:

Error: src/app/example/example.component.html:1:1 - error NG8001: 'mat-checkbox' is not a known element:
1. If 'mat-checkbox' is an Angular component, then verify that it is part of this module.
2. If 'mat-checkbox' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.

1 <mat-checkbox></mat-checkbox>
  ~~~~~~~~~~~~~~

  src/app/example/example.component.ts:5:16
    5   templateUrl: './example.component.html',
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~
    Error occurs in the template of component ExampleComponent.




× Failed to compile.

The error suggests that an implementation for mat-checkbox must be provided in AppModule, and that it's too late to fill it in in BrowserAppModule as the docs suggest.

(Note: there are a couple of other typos in the snippets ignoring the issues mentioned above - app.module.ts and browser-app.module.ts are missing their classes, server.app.module.ts should probably be renamed server-app.module.ts, and a main.ts snippet should probably be added to illustrate that it should point to browser-app.module.ts)

🔥 Exception or Error

See above

🌍 Your Environment


Angular CLI: 13.1.1                                             
Node: 14.17.6                                                   
Package Manager: npm 7.21.0                                     
OS: win32 x64                                                   
                                                                
Angular: 13.1.0                                                 
... animations, cdk, common, compiler, compiler-cli, core, forms
... material, platform-browser, platform-browser-dynamic        
... platform-server, router                                     
                                                                
Package                         Version                         
---------------------------------------------------------       
@angular-devkit/architect       0.1301.1                        
@angular-devkit/build-angular   13.1.1                          
@angular-devkit/core            13.1.1                          
@angular-devkit/schematics      13.1.1
@angular/cli                    13.1.1
@nguniversal/builders           13.0.1
@nguniversal/express-engine     13.0.1
@schematics/angular             13.1.1
rxjs                            7.4.0
typescript                      4.5.4

benelliott avatar Dec 15 '21 10:12 benelliott

Hi @alan-agius4 , do you have any ideas for a possible workaround to this issue?

benelliott avatar Jan 04 '22 12:01 benelliott

This is expected and not a Universal issue.

The problem is that MatCheckboxModule is not imported in correct module . The MatCheckboxModule needs to be imported in AppModule since it is required by components in that module.

alan-agius4 avatar Jan 13 '22 09:01 alan-agius4

Hi @alan-agius4 , thanks for the response.

In the demo, the MatCheckboxModule is being imported to provide the browser implementation as per the instructions in https://github.com/angular/universal/blob/master/docs/gotchas.md.

That documentation suggests the following approach:

  • ExampleComponent uses server-incompatible LibraryComponent
  • AppModule declares ExampleComponent
  • BrowserAppModule imports AppModule and LibraryModule (which provides browser implementation of LibraryComponent)
  • LibraryShimComponent is server-compatible shim for LibraryComponent
  • ServerAppModule imports AppModule and declares LibraryShimComponent

Likewise, my demo does the following:

  • ExampleComponent uses (for the sake of argument, server-incompatible) MatCheckboxComponent
  • AppModule declares ExampleComponent
  • BrowserAppModule imports AppModule and MatCheckboxModule (which provides browser implemenation of MatCheckboxComponent)

At this point, we reach the compilation error.

The subject of my issue is not the compilation error itself, but that the documentation suggests this invalid approach to work around server-incompatible library components. Your statement that "The MatCheckboxModule needs to be imported in AppModule since it is required by components in that module" is the equivalent of saying that in the documentation example, "The LibraryModule needs to be imported in AppModule since it is required by components in that module", which defeats the point of the example.

Is the documentation therefore wrong? If so, how do we use server-incompatible libraries with Universal?

benelliott avatar Jan 13 '22 11:01 benelliott

Oh I see the problem now.

The documentation does seem to be wrong in this case. In general, I think the best action would be to inform the library authors that their Angular library is not SSR compatible.

Will need to look into alternatives, as right of the bet I don't exactly know a good way around this. Maybe @CaerusKaru knows?

alan-agius4 avatar Jan 13 '22 11:01 alan-agius4

Thanks - out of interest, do you know whether it is a regression? I would imagine that those instructions did at one point work for shimming library components, but the ecosystem seeems to have become stricter as of Angular 13.

benelliott avatar Jan 13 '22 12:01 benelliott

@benelliott I'll take a closer look at that question; I was the original author so I'll re-evaluate the strategy in a fresh environment.

CaerusKaru avatar Jan 14 '22 16:01 CaerusKaru

I've confirmed things are exactly as you've reported, and that the workaround is to instead declare the components at the upper level module (browser and server, instead of just app). Obviously this isn't what we had hoped for in terms of usability, and this may have changed since Ivy was published. I'll check in with the framework subteam next week after the holiday to see what's going on. It may be that this is the new normal, and we'll have to update our guidance accordingly.

CaerusKaru avatar Jan 17 '22 01:01 CaerusKaru

Thanks! Would you mind describing the workaround in a bit more detail?

benelliott avatar Jan 17 '22 10:01 benelliott

After talking this over a little more, another option is to have a separate environment file for the server, and configure it in your angular.json. So if you have something like:

export const environment = {
  isServer: true,
};

You could then have your AppModule as something like:

@NgModule({
  imports: [MatCheckboxModule],
  declarations: [ environment.isServer ? ServerComponent : BrowserComponent ],
})
export class AppModule {}

Then you wouldn't need a BrowserAppModule to distinguish from ServerAppModule; it would be unified, however thinly at the AppModule level.

We're still thinking about a design that would allow us to tree-shake out components depending on platform preference, but that would require a lot more effort, not to mention an RFC, to get it done the way we'd expect.

CaerusKaru avatar Jan 20 '22 05:01 CaerusKaru

Nice - I didn't know the Angular compiler allowed ternaries inside NgModule metadata like that. I will give it a try at some point.

benelliott avatar Jan 20 '22 09:01 benelliott

export const environment = {
  isServer: true,
};

Hello, I am running into the exact same issue as the OP. I am trying to use a third-party library, which works fine in development server but, when I try to run it in SSR mode, I either get an error window not defined or I get this error:

If 'apx-chart' is an Angular component, then verify that it is part of this module.

I tried this approach but isServer will always be true if I understand correctly? I am importing the environment.server.ts file in my app.module.ts file and then doing the checks, I have also configured it in my angular.json under server and fileReplacements file but I don't understand how it changes from browser to server. Could you help me with this, please? I will really appreciate it.

UPDATE 1

It finally worked! Thank you very much for this solution. I have been trying to solve this issue for 12 days now but I didn't check for the environment, I have no idea how it worked but I think I just added BrowserModule.withServerTransition({ appId: 'serverApp' }) in app.server.module.ts file and removed the AppModule imports from it, I also don't have a app-browser.module.ts file, it's everything in app-module.ts file

UPDATE 2

It worked in a new project I created for testing purposes but I can't get it to work on another project. I have no idea how I managed to make it work tho. I am trying to replicate the steps.

UPDATE 3

After testing over and over I came up with this:

app.module.ts

import { NgModule } from '@angular/core';
import { ThirdPartyLibraryModule } from 'ng-third-party';
import { AppComponent } from './app.component';
import { BrowserModule } from '@angular/platform-browser';

@NgModule({
  declarations: [
    AppComponent //Component that uses third party library
  ],
  imports: [
    BrowserModule.withServerTransition({ appId: 'serverApp' }),
    ThirdPartyLibraryModule //Third Party Library Module here
  ],
  bootstrap:[AppComponent]
})
export class AppModule { }


app.server.module.ts

import { NgModule } from '@angular/core';
import { AppModule } from './app.module';
import { ServerModule } from '@angular/platform-server';
import { AppShimComponent } from './app-shim.component'; //In case you implement the third library in the AppComponent
import { BrowserModule } from '@angular/platform-browser';

@NgModule({
  imports: [ServerModule, BrowserModule.withServerTransition({ appId: 'serverApp' })],
  declarations: [],
  
   //AppShimComponent with nothing in it, just the app-root selector and empty template 
  //* or you can import the regular AppComponent) 
  bootstrap: [AppShimComponent] 
})

export class AppServerModule {}

In my app.component.html file I have this:

  <third-party
    [config]="config.libraryConfig"
  ></third-party>

* If the app implements the third party in another component that is not AppComponent (which is the case for most applications) you can use the regular AppComponent in bootstrap in app.server.module.ts file with a check on <router-outlet> element, in app.component.html file or simply use AppShimComponent (AppComponent just the skeleton)

If you don't check for the environment or don't use a skeleton, you will get this error:

NullInjectorError: R3InjectorError(AppServerModule)[ChildrenOutletContexts -> ChildrenOutletContexts -> ChildrenOutletContexts]: 
  NullInjectorError: No provider for ChildrenOutletContexts!

So you could try do one of the following:

Method 1

Create a AppShimComponent file and import and boostrap it in app.server.module.ts also import BrowserModule with server transition:

import {Component} from '@angular/core';

@Component({
  selector: 'app-root',
  template: '',

})
export class AppShimComponent { }

Method 2:

Check if platform is browser., in your app.component.html file:

<div *ngIf="browser">
  <router-outlet></router-outlet>
</div>

and in your app.component.ts file you do this:

import {Component, PLATFORM_ID, Inject} from '@angular/core';
import { isPlatformBrowser } from '@angular/common';

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.css']
})

export class AppComponent {
  public platform: string = '';
  public browser: boolean = true;

  constructor(@Inject(PLATFORM_ID) platformId: string) {
    this.platform = platformId;
  }

  ngOnInit(): void {
    this.browser = isPlatformBrowser(this.platform)
  }

}

So the problem is that I was implementing the third party library in AppComponent which is bootstraped by AppServerModule that will be included in ther server so that will break the app because there will be client side code running on server side. So either create a AppShimComponent file or implement the third party library in another component. Also, no ngIf was required and I didn't have to create any aditional module, at least in my case, it just worked, but if the app has therouter-outlet element in AppComponent a check needs to be made or create an AppShimComponent for app.server.module to bootstrap .

To summarize, the major change occurs in app.server.module.ts that, instead of importing AppModule, will import only ServerModule and BrowserModule with server transition, like this: imports: [ServerModule, BrowserModule.withServerTransition({ appId: 'serverApp' })] althoughAppModule still has to be imported in app.server.module for it to work (but not in NgNodules) and then it will work without having to do any additional modifications, like creating new modules, changing angular.json file or checking in which environment we are in.

tguimmaraess avatar Mar 08 '22 16:03 tguimmaraess

After talking this over a little more, another option is to have a separate environment file for the server, and configure it in your angular.json. So if you have something like:

export const environment = {
  isServer: true,
};

You could then have your AppModule as something like:

@NgModule({
  imports: [MatCheckboxModule],
  declarations: [ environment.isServer ? ServerComponent : BrowserComponent ],
})
export class AppModule {}

Then you wouldn't need a BrowserAppModule to distinguish from ServerAppModule; it would be unified, however thinly at the AppModule level.

We're still thinking about a design that would allow us to tree-shake out components depending on platform preference, but that would require a lot more effort, not to mention an RFC, to get it done the way we'd expect.

Has this actually been tested? I tried this but the problem was that BrowserComponent was still trying to be compiled when generating the server bundles because it still has to be imported by AppModule. This generated a bunch of "isn't a known property" errors due to unknown elements in BrowserComponent's template.

@CaerusKaru can you please elaborate on the work around mentioned here?

s-moran avatar Oct 15 '23 21:10 s-moran