manfredsteyer / angular-oauth2-oidc

Support for OAuth 2 and OpenId Connect (OIDC) in Angular.
MIT License
1.88k stars 687 forks source link

Error on npm run serve:ssr #773

Open fluxc0d3r opened 4 years ago

fluxc0d3r commented 4 years ago

Describe the bug I'm trying to build and serve a local app in ssr mode but npm run server:ssr crash.

To Reproduce Steps to reproduce the behavior:

  1. npm install angular-oauth2-oidc --save
  2. add OAuthModule.forRoot() on AppModule
  3. npm run build:ssr && npm run serve:ssr or npm run dev:ssr

Expected behavior Node Express server listening on http://localhost:4000

Additional context ReferenceError: Document is not defined at Module../node_modules/angular-oauth2-oidc/fesm2015/angular-oauth2-oidc.js

mrfoster commented 4 years ago

A temporary workaround is to use version 8.0.2 if that's possible.

PandaaAgency commented 4 years ago

any news about this issue ? i'm using angular 9.1 and i'm facing the same problem ..

kristofdegrave commented 4 years ago

I think that the document used here https://github.com/manfredsteyer/angular-oauth2-oidc/blob/ff82dd9b7fc81a1a001730a4013c639061fe57e9/projects/lib/src/oauth-service.ts#L982-L1008 here https://github.com/manfredsteyer/angular-oauth2-oidc/blob/ff82dd9b7fc81a1a001730a4013c639061fe57e9/projects/lib/src/oauth-service.ts#L1180 and here https://github.com/manfredsteyer/angular-oauth2-oidc/blob/ff82dd9b7fc81a1a001730a4013c639061fe57e9/projects/lib/src/oauth-service.ts#L1302-L1338 should be prefixed by this.document. The DOCUMENT is injected correctly, but not applied on all document references in the file. https://github.com/manfredsteyer/angular-oauth2-oidc/blob/ff82dd9b7fc81a1a001730a4013c639061fe57e9/projects/lib/src/oauth-service.ts#L115

PandaaAgency commented 4 years ago

hi @kristofdegrave , thanks for taking a loot at it πŸš€ event if your fix seems necessary to avoid later issues, it doesn't seem to fix the entire problem..

I used your fix on my local fork and test on my app with a little npm link and still have the same issue. It seems, imo, that the Document isn't injected at all ..

Documents\Workspace\poc-front\dist\activities\server\main.js:2540
        Document])
        ^

ReferenceError: Document is not defined
    at Module.../../poc/angular-oauth2-oidc/dist/lib/__ivy_ngcc__/fesm2015/angular-oauth2-oidc.js (Documents\Workspace\poc-front\dist\activities\server\main.js:2540:9)
kristofdegrave commented 4 years ago

Seeing your error are you using this library at server side? Our is this still your client that is giving the error?

kristofdegrave commented 4 years ago

There seems to be a setDocument method to set the DOCUMENT in Ivy, normally Ivy takes care of this, but it seems in your case not. https://github.com/angular/angular/blob/e511bfcab586a94cdae1491e0ec194141445578b/packages/core/src/render3/interfaces/document.ts

It is a private function exposed in the core package, maybe try to set the document yourself. https://github.com/angular/angular/blob/fb92f5de1adc8dccbb947a09b34cb1bcc6b41e7f/packages/core/src/core_render3_private_export.ts#L257

PandaaAgency commented 4 years ago

Seeing your error are you using this library at server side? Our is this still your client that is giving the error?

this error occurs at build time when building with ssr (npm run build:ssr or npm run dev:ssr)

There seems to be a setDocument method to set the DOCUMENT in Ivy, normally Ivy takes care of this, but it seems in your case not. https://github.com/angular/angular/blob/e511bfcab586a94cdae1491e0ec194141445578b/packages/core/src/render3/interfaces/document.ts

It is a private function exposed in the core package, maybe try to set the document yourself. https://github.com/angular/angular/blob/fb92f5de1adc8dccbb947a09b34cb1bcc6b41e7f/packages/core/src/core_render3_private_export.ts#L257

i will try to check if i can manage to have something that works this, thanks @kristofdegrave

kristofdegrave commented 4 years ago

What is in your main.ts file? How are you bootstrapping your appmodule?

if you are using platformDynamicServer to bootstrap, try something like this:

platformDynamicServer([{
           provide: INITIAL_CONFIG,
           useValue: {document: '<html><head></head><body><app></app></body></html>'}
         }]);

This comes from one of the angular test. I asume that the document should contain your index.html representation. https://github.com/angular/angular/blob/a0d16dcfea80f089379e4f6b9a4da68790cc1436/packages/platform-server/test/integration_spec.ts#L469-L482

EmmanuelDemey commented 4 years ago

We setup the project with the Nest plugin. In our main.ts, we use platformBrowserDynamic.

We tried adding the INITIAL_CONFIG option, we still have the error.

import { enableProdMode } from '@angular/core';
import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';

import { AppModule } from './app/app.module';
import { environment } from './environments/environment';
import { INITIAL_CONFIG } from '@angular/platform-server';

if (environment.production) {
  enableProdMode();
}

document.addEventListener('DOMContentLoaded', () => {
  platformBrowserDynamic([
    {
      provide: INITIAL_CONFIG,
      useValue: {
        document: '<html><head></head><body><app></app></body></html>',
      },
    },
  ])
    .bootstrapModule(AppModule)
    .catch((err) => console.error(err));
});
EmmanuelDemey commented 4 years ago

I have just create a new ssr projet with nest. And I inject the DOCUMENT token in my AppComponent. And in this use case, I do not have any errors. :s

kristofdegrave commented 4 years ago

Did you import the BrowserModule in your AppModule? This can also be a reason. This defines a document ... https://github.com/angular/angular/blob/b01084910266deaa7d8e4424398062e6905199e8/packages/platform-browser/src/browser.ts#L42

PandaaAgency commented 4 years ago

Did you import the BrowserModule in your AppModule? This can also be a reason. This defines a document ... https://github.com/angular/angular/blob/b01084910266deaa7d8e4424398062e6905199e8/packages/platform-browser/src/browser.ts#L42

yes it is :

@NgModule({
  declarations: [AppComponent],
  imports: [
    BrowserModule.withServerTransition({ appId: 'serverApp' }),
....
kristofdegrave commented 4 years ago

I have just create a new ssr projet with nest. And I inject the DOCUMENT token in my AppComponent. And in this use case, I do not have any errors. :s

Can you see what the difference is in setup between the original and the new one?

EmmanuelDemey commented 4 years ago

@kristofdegrave In fact, I have an error. It was not very clear last time I checked this issue.

this.debug is not a function

Not very useful :s

The error we have with oidc is about Document with a capitalized D. So it means the DOM object Document is missing. Am I right ? That's normal because we are on the service side. So this class do not exist.

But I can not find any documentation about this issue on the Angular Universal documentation :s . Should we import an extra NPM module like domino ?

kristofdegrave commented 4 years ago

The error we have with oidc is about Document with a capitalized D. So it means the DOM object Document is missing. Am I right ? That's normal because we are on the service side. So this class do not exist.

Or it would mean that he has issues with the type Document. This is a type that should be defined inside typescript lib

But I can not find any documentation about this issue on the Angular Universal documentation :s . Should we import an extra NPM module like domino ?

About this subject I know nothing :)

Exocomp commented 4 years ago

I'm getting the error as well, working with angular-oauth2-oidc v9 (Angular 9.1.0). At the moment now sure why.

ReferenceError: Document is not defined
    at Module../node_modules/angular-oauth2-oidc/fesm2015/angular-oauth2-oidc.js (C:\***\dist\***\server\main.js:206143:9)

The line it is referring to in main.js:

OAuthService = Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__decorate"])([ 
...
...
[_angular_core__WEBPACK_IMPORTED_MODULE_1__["NgZone"],
        _angular_common_http__WEBPACK_IMPORTED_MODULE_3__["HttpClient"],
        OAuthStorage,
        ValidationHandler,
        AuthConfig,
        UrlHelperService,
        OAuthLogger,
        HashHandler,
        Document])      <--------
], OAuthService);
kristofdegrave commented 4 years ago

Looks like he can't resolve the type Document normally this should be part of the typescript library.

Seems in this post they get the same issue (https://stackoverflow.com/questions/50517138/ng-bootstrap-referenceerror-document-is-not-defined)

But if that is the case I would solve it like that, because angular provides it's own solution using the platform-server (https://angular.io/api/platform-server). I would advise to use the platformDynamicServer instead of platformBrowserDynamic in your main.server.ts

Exocomp commented 4 years ago

I created a new Angular 9 project and added SSR, keeping everything default.

Then as a test, I injected the DOCUMENT token in AppComponent and use it in ngOnInit(). I could not reproduce the problem.

However, as soon as I added angular-oauth2-oidc and OAuthModule.forRoot in AppModule I got the error (Document is not defined) on build.

Seems to me there is something specific with angular-oauth2-oidc. If I get some more time I will do some more testing.

Exocomp commented 4 years ago

Here is a workaround fix, removing the Document type from the inject token doesn't throw the error.

At the moment not sure why, what do you think?

See my fork here https://github.com/Exocomp/angular-oauth2-oidc/blob/master/projects/lib/src/oauth-service.ts#L115

However, this creates the side effect of not being able to use intellisense for document.

kristofdegrave commented 4 years ago

Maybe try the following: Create a new interface extending Document Document2 for example and export this one in the public API also use Document2 in the authservice constructor. This way the type gets exported and maybe this solves the issue

kristofdegrave commented 4 years ago

Another try, does your ssr tsconfig contain the dom value in the lib property? Inspect the whole tree for this ​"​lib​"​:​ [​"​es2015​"​, ​"​dom​"​],

https://basarat.gitbook.io/typescript/type-system/lib.d.ts#lib-option

Exocomp commented 4 years ago

By default the Angular CLI uses:

"lib": [
  "es2018",
  "dom"
]
EmmanuelDemey commented 4 years ago

I have just added this in my tsconfig.server.json, and I still have the error

"lib": ["DOM", "ES2015"],
gmiklich commented 4 years ago

I have the same issue, but as MrFoster pointed out, reverting back to 8.0.2 worked for now.

elgerm commented 4 years ago

Same here, can't go back to 8.x because I'm on angular ivy / 9 and I can't release this because we're using SSR

elgerm commented 4 years ago

In the mean time, I created a new library with the Document type removed (thanks @Exocomp ).

https://www.npmjs.com/package/@elgerm/angular-oauth2-oidc

It's based on the latest master but the versioning is still on 9.1. I've tested several stuff that relies on document like refresh token and it all seems to work.

Now at least I can use it in production.

gmiklich commented 4 years ago

Dumb question, but out of curiosity, is there something in particular that doesn't work when using 8.x? I have a similar requirement of using this library with angular 9 and SSR, and it seems to be working ok. You actually have a workable solution, which is the way to go, but again I was just curious. I know we're supposed to use the matching version, but I'm not sure how to go about finding out what would break, or at least not be guaranteed to work, if using an older package version with the newer angular version.

elgerm commented 4 years ago

Not sure, I started using it with v9, then tried 8.0.3 and got lots of errors. So that made me fork and β€œfix” the v9 version

On Thu, 7 May 2020 at 18:43, mikg2003 notifications@github.com wrote:

Dumb question, but out of curiosity, is there something in particular that doesn't work when using 8.x? I have a similar requirement of using this library with angular 9 and SSR, and it seems to be working ok. You actually have a workable solution, which is the way to go, but again I was just curious. I know we're supposed to use the matching version, but I'm not sure how to go about finding out what would break, or at least not be guaranteed to work, if using an older package version with the newer angular version.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/manfredsteyer/angular-oauth2-oidc/issues/773#issuecomment-625368127, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAW3SSSY22DXVA3APMXRWLTRQLQMTANCNFSM4LULXJAQ .

PandaaAgency commented 4 years ago

still no news about this issue ? i've tried some fixes on my side but without any luck yet ..

EmmanuelDemey commented 4 years ago

Yes no news :(

jrmcdona commented 4 years ago

I don't use angular-oauth2-oidc but I am getting this error anyway. So it is something else for me. But was I led to this page via google search for "this.debug is not a function"

JohannesHuster commented 4 years ago

We were able to work around this, for now, by adding Document to the global object when running on the server. For example, add

(global as any).Document = (global as any).Document || {};

at the beginning of your main.server.ts. Be careful to add this in a file that is included only on the server.

I have also added a PR for this #853.

PandaaAgency commented 4 years ago

We were able to work around this, for now, by adding Document to the global object when running on the server. For example, add

(global as any).Document = (global as any).Document || {};

at the beginning of your main.server.ts. Be careful to add this in a file that is included only on the server.

I have also added a PR for this #853.

obviously not a long term fix, but this workaround works well on our side too, well done @JohannesHuster

jgcarmona-com commented 4 years ago

Hi there,

I was evaluating whether to use this library in my solution because I am using (the) other oidc client option and facing some weird issues regarding token storage at server side (and protected modules CanLoad)...

Well, for all of you that are facing issues with document, window and any other browser APIs here is my advice:

As an example (a working example indeed):

import { APP_BASE_HREF } from '@angular/common';
import '@angular/localize/init';
import { ngExpressEngine } from '@nguniversal/express-engine';
import * as express from 'express';
import { existsSync } from 'fs';
import { join } from 'path';
import 'zone.js/dist/zone-node';

// THIS FIX MOST OF THE COMMON ISSUES WITH SSR:
const enDistFolder = join(process.cwd(), 'dist/browser/en');

// Emulate browser APIs
const domino = require('domino');
const fs = require('fs');
const templateA = fs.readFileSync(join(enDistFolder, 'index.html')).toString();

const win = domino.createWindow(templateA);
console.log('win');
win.Object = Object;
console.log('Object');
win.Math = Math;
console.log('Math');

global['window'] = win;
global['document'] = win.document;
global['Event'] = win.Event;
console.log('declared Global Vars....');

/** I need to avoid sorting this line */
// USE CTRL+P -> SAVE WITHOUT FORMATTING
/// --> import { AppServerModule } from './main.server';
import { AppServerModule } from './main.server';
/// --> import { AppServerModule } from './main.server';///
/** I need to avoid sorting this line */

// The Express app is exported so that it can be used by serverless Functions.
export function app() {
  const server = express();
  const distFolder = join(process.cwd(), 'dist/browser');
  const indexHtml = existsSync(join(distFolder, 'index.original.html')) ? 'index.original.html' : 'index';

  // Our Universal express-engine (found @ https://github.com/angular/universal/tree/master/modules/express-engine)
  server.engine('html', ngExpressEngine({
    bootstrap: AppServerModule,
  }));

  server.set('view engine', 'html');
  server.set('views', distFolder);

  // Example Express Rest API endpoints
  // server.get('/api/**', (req, res) => { });
  // Serve static files from /browser
  server.get('*.*', express.static(distFolder, {
    maxAge: '1y'
  }));

  server.use('/robots.txt', express.static('/en/robots.txt'));
  server.use('/ads.txt', express.static('/en/ads.txt'));

  // // // All regular routes use the Universal engine
  // // server.get('*', (req, res) => {
  // //   res.render(indexHtml, { req, providers: [{ provide: APP_BASE_HREF, useValue: req.baseUrl }] });
  // // });

  // All regular routes use the Universal engine
  server.get('*', (req, res) => {
    // this is for i18n
    const supportedLocales = ['en', 'es'];
    const defaultLocale = 'es';
    const matches = req.url.match(/^\/([a-z]{2}(?:-[A-Z]{2})?)\//);

    // check if the requested url has a correct format '/locale' and matches any of the supportedLocales
    const locale = (matches && supportedLocales.indexOf(matches[1]) !== -1) ? matches[1] : defaultLocale;

    res.render(`${locale}/index`, { req });

    // // AS A POC WE WANT TO RENDER ENG VERSION ONLY:
    // // res.render(`en/index`, { req });
  });

  return server;
}

function run() {
  const port = process.env.PORT || 4000;

  // Start up the Node server
  const server = app();
  server.listen(port, () => {
    console.log(`Node Express server listening on http://localhost:${port}`);
  });
}

// Webpack will replace 'require' with '__webpack_require__'
// '__non_webpack_require__' is a proxy to Node 'require'
// The below code is to ensure that the server is run only when not requiring the bundle.
declare const __non_webpack_require__: NodeRequire;
const mainModule = __non_webpack_require__.main;
const moduleFilename = mainModule && mainModule.filename || '';
if (moduleFilename === __filename || moduleFilename.includes('iisnode')) {
  run();
}

export * from './main.server';

Note that this code is using i18n but I left the original request handler.

I hope it helps, I spent a huge amount of time fixing my own issues after Angular 9 migration.

JohannesHuster commented 4 years ago

Thank you sharing your solution with browser APIs @JuanGarciaCarmona :-) Reading through your answer and this issue again, I realized that it might be helpful to separate the 3 different case-sensitive spellings of "Document" used in this issue:

The problem described in this issue has to do with the last spelling ("Document") and seems to have something to do with Dependency Injection in Ivy. When compiling TypeScript to JavaScript this type information would normally not be represented in any way in the JS output. However, in the output from the Angular Compiler (at least when enabling Ivy) there is a reference to Document (Comments are mine):

// Universal main.js
// ...
OAuthService = Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__decorate"])([ Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__param"])(2, Object(_angular_core__WEBPACK_IMPORTED_MODULE_1__["Optional"])()),
    Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__param"])(3, Object(_angular_core__WEBPACK_IMPORTED_MODULE_1__["Optional"])()),
    Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__param"])(4, Object(_angular_core__WEBPACK_IMPORTED_MODULE_1__["Optional"])()),
    Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__param"])(7, Object(_angular_core__WEBPACK_IMPORTED_MODULE_1__["Optional"])()),
    Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__param"])(8, Object(_angular_core__WEBPACK_IMPORTED_MODULE_1__["Inject"])(_angular_common__WEBPACK_IMPORTED_MODULE_2__["DOCUMENT"])),
    Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__metadata"])("design:paramtypes", [_angular_core__WEBPACK_IMPORTED_MODULE_1__["NgZone"],
        _angular_common_http__WEBPACK_IMPORTED_MODULE_3__["HttpClient"],
        OAuthStorage,
        ValidationHandler,
        AuthConfig,
        UrlHelperService,
        OAuthLogger,
        HashHandler,
        Document]) // --> Reference Error since Document is never declared (where this scope could access it)
], OAuthService);
// ...

This works for the references above Document, because they are declared in a surrounding scope. This might be an Angular bug, but so far I haven't been able to reproduce the problem with a clean Angular library. I might find the time to look into this more and would then open an issue in the Angular repo, if I manage to reproduce the behavior.

manfredsteyer commented 3 years ago

Thanks for all the information and solutions provided in this thread.

Perhaps this also helps: Meanwhile, the Angular Universal team published a new API using jsDOM. This makes using libs that have been primarily written for browser usage easier. You can try this new API out using ng add @nguniversal/common.