angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.76k stars 11.98k forks source link

@angular-devkit/build-angular:server with optimization: true fails in 16.1.4. 16.1.1 works well #25529

Open juanmaldonadodev opened 1 year ago

juanmaldonadodev commented 1 year ago

Command

run

Is this a regression?

The previous version in which this bug was not present was

16.1.1

Description

Exception appears on run main.js with node 16 after build with optimization using @angular-devkit/build-angular@16.1.4 with @angular-devkit/build-angular@16.1.1 works well

Minimal Reproduction

1 buid the project: ng build --configuration=production && ng run app:server:production Works correctly 2 Run the main.js present exception node dist/app/server/main.js

TypeError: Right-hand side of 'instanceof' is not an object
    at insertElement (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:490276)
    at insertHTMLElement (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:489998)
    at before_head_mode (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:527950)
    at HTMLParser.htmlparser.insertToken (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:489022)
    at emitSimpleTag (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:494203)
    at data_state (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:494241)
    at scanChars (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:485784)
    at Object.parse (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:483851)
    at Object.exports2.createDocument (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:721803)
    at Object.exports2.createWindow (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:722306)

The problem disappears if in angular.json optimization is set to false --> Not a real fix or if downgrade to "@angular-devkit/build-angular": "^16.1.1",

Final configuration: `Angular CLI: 16.1.4 Node: 16.20.0 Package Manager: npm 8.19.4 OS: darwin x64

Angular: 16.1.4 ... animations, cdk, cli, common, compiler, compiler-cli, core ... forms, language-service, localize, material ... platform-browser, platform-browser-dynamic, platform-server ... router

Package Version

@angular-devkit/architect 0.1601.4 @angular-devkit/build-angular 16.1.4 @angular-devkit/core 16.1.4 @angular-devkit/schematics 16.1.4 @nguniversal/builders 16.1.1 @nguniversal/express-engine 16.1.1 @schematics/angular 16.1.4 rxjs 7.8.1 typescript 5.1.6`

Exception or Error

TypeError: Right-hand side of 'instanceof' is not an object
    at insertElement (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:490276)
    at insertHTMLElement (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:489998)
    at before_head_mode (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:527950)
    at HTMLParser.htmlparser.insertToken (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:489022)
    at emitSimpleTag (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:494203)
    at data_state (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:494241)
    at scanChars (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:485784)
    at Object.parse (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:483851)
    at Object.exports2.createDocument (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:721803)
    at Object.exports2.createWindow (/Users/juanmaldonado/dev/ekilu-web-16/dist/app/server/main.js:1:722306)

Your Environment

`Angular CLI: 16.1.4
Node: 16.20.0
Package Manager: npm 8.19.4
OS: darwin x64

Angular: 16.1.4
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, localize, material
... platform-browser, platform-browser-dynamic, platform-server
... router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1601.4
@angular-devkit/build-angular   16.1.4
@angular-devkit/core            16.1.4
@angular-devkit/schematics      16.1.4
@nguniversal/builders           16.1.1
@nguniversal/express-engine     16.1.1
@schematics/angular             16.1.4
rxjs                            7.8.1
typescript                      5.1.6`

Anything else relevant?

No response

juanmaldonadodev commented 1 year ago

More users affected in this link: https://stackoverflow.com/questions/76611780/angular-ssr-error-right-hand-side-of-instanceof-is-not-an-object/76668748#76668748

JeanMeche commented 1 year ago

It's this line that is throwing : https://github.com/angular/domino/blob/aa8de3486307f57a518b4b0d9e5e16d9fbd998d1/lib/HTMLParser.js#L2645C59-L2645C60

juanmaldonadodev commented 1 year ago

It's this line that is throwing : https://github.com/angular/domino/blob/aa8de3486307f57a518b4b0d9e5e16d9fbd998d1/lib/HTMLParser.js#L2645C59-L2645C60

Do you think that the problem is in domino library and not in the build?

Routhinator commented 1 year ago

Hmm, I've been banging my head against this for a couple of days, and @juanico18 's comment made me try removing domino from my server.ts, and now my SSR server starts without this exception.

..my app fails overall of course because I need to provide window for some things in my app, but that does seem to be the right track for this issue.

Routhinator commented 1 year ago

As a workaround for anyone stuck with this, I dropped in ssr-window instead of domino and things seem to be working now. It's an alternative to downgrading devkit for now.

Example:

import { getWindow, getDocument } from 'ssr-window';
;(global as any).window = getWindow()
;(global as any).document = getDocument()
;(global as any).navigator = {
  language: 'en',
}

I'm a little concerned about this as a long-term workaround as the ssr-window project has been idle longer than domino, but I note that domino itself has also been lacking updates for 2 years now.

Ultimately I should probably get on with doing platform checks to gate loading anything dependent on window/document/navigator and remove the need for this.

Routhinator commented 1 year ago

Eh, I take it back. domino is hard to replace. It's implementation of DOMParser is more functionally complete than anything else.

ahmadallw commented 1 year ago

any new updated on this issue?

deni-rhn commented 1 year ago

@Routhinator @JeanMeche have you resolved this problem? I have been stuck for a couple of days thinking to resolve this.

Routhinator commented 1 year ago

My resolution was to rewrite any components that were using non angular safe methods of using document or window etc.

IE: use Document from angular/core, or if you're looking to get an element use ViewChild and a setter, etc.

I've ripped out libs that don't do this or if they are user-only contexts, wrapped the loading of the module in a platform check.

manduriomane commented 1 year ago

Hi all, I have the same problem. Are there updates?

chakrachi commented 1 year ago

I'm experiencing the same issue with post versions @angular-devkit/build-angular v16.1.1, hoping for a fix after the new 16.2.0 RCs?

hiepxanh commented 1 year ago

have the same issue with 16.2.0 official version on production build :(

hiepxanh commented 1 year ago

I think this should be top priority fixing issue now. Since it breaks application build workflow

alan-agius4 commented 1 year ago

Thanks for reporting this issue. However, you didn't provide sufficient information for us to understand and reproduce the problem. Can you setup a minimal repro please?

You can read here why this is needed. 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.

hiepxanh commented 1 year ago

aw sorry sir, I though that one provided it. Let me do that. I'm so sorry

Wycza commented 1 year ago

It's because of the domino package. To reproduce it:

  1. Create a new project
  2. Add ng add @nguniversal/express-engine
  3. npm install domino --save
  4. Add this to server.ts:
 const distFolder = join(process.cwd(), 'dist/YOUR_PROJECT_NAME/browser');
  const window = domino.createWindow(template);
  global['window'] = window;
  global['document'] = window.document;
  1. npm run build:ssr and then npm run server:ssr

This is how it should look like:

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

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

  const template = fs
  .readFileSync(path.join(join(process.cwd(), 'dist/bug-repro/browser'), 'index.html'))
  .toString();

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

  // This cause the bug
  const window = domino.createWindow(template);
  global['window'] = window;
  global['document'] = window.document;

  [...]
alan-agius4 commented 1 year ago

@Wycza, thanks I managed to replicate it.

chakrachi commented 1 year ago

allowing for SSR hydration exacerbates the problem, perhaps you can find a link with these issues

djgovani commented 1 year ago

Today converted the app (v16.2.0) to SSR and then try to build and serve and got the same error. As Wycza said in his comment I'm also using domino. I did Google and found this issue. Hope this will fix soon.

krivanek06 commented 1 year ago

+1 on this

vc-hitesh commented 1 year ago

+1 on this

REPTILEHAUS commented 1 year ago

Hitting this also v16.2.1

D-Biela commented 1 year ago

Hitting this also v16.2.1

Same. On 16.1.1 it works fine. Im also using domino with angular universal.

ahmadallw commented 1 year ago

@D-Biela you are not facing the same issue?

D-Biela commented 1 year ago

@ahmadallw Im facing the same issue with @angular-devkit/build-angular v16.2.1, but as mentioned in this thread, downgrading to @angular-devkit/build-angular 16.1.1 solves the problem.

alucardu commented 1 year ago

Just upgraded Angular to version 16.2.4 and also running into this error.

manduriomane commented 1 year ago

Hi to all. Are there updates?

ahmadallw commented 1 year ago

Hi all - Any new updates?

igordanilovski commented 11 months ago

Same issue for me.

geferon commented 11 months ago

For anyone that runs into this issue but still needs to use Domino, Angular has a forked Domino repository which works with v17s SSR: https://github.com/angular/domino It's not on NPM though, so you will have to install it through git.

dchicurel commented 11 months ago

+1 facing this issue.

Can the forked domino be used with Angular 16 ? could this be released as a standalone Angular-domino package?

dchicurel commented 11 months ago

could this be released as a standalone Angular-domino package?

I guess this answers the question: https://github.com/angular/angular/blob/ec2d6e7b9c2b386247d1320ee89f8e3ac5e5a0dd/package.json#L118C5-L118C96

Tried that with Angular 16.2 and seems to be working fine! "domino": "https://github.com/angular/domino.git#9e7881d2ac1e5977cefbc557f935931ec23f6658",

JeanMeche commented 11 months ago

Angular has been using its own fork of domino since v16, cf this commit

ndr commented 10 months ago

It's this line that is throwing : https://github.com/angular/domino/blob/aa8de3486307f57a518b4b0d9e5e16d9fbd998d1/lib/HTMLParser.js#L2645C59-L2645C60

I made a domino fork, made a workaround to fix this line throwing an error and linked my "domino" to the commit with fix in my package.json:

"domino": "https://github.com/ndr/domino.git#87c3ef9d60965ded1e713893c3b7c32a421958b1"

For me it worked (tried with angular 17)

D-Biela commented 9 months ago

It's this line that is throwing : https://github.com/angular/domino/blob/aa8de3486307f57a518b4b0d9e5e16d9fbd998d1/lib/HTMLParser.js#L2645C59-L2645C60

I made a domino fork, made a workaround to fix this line throwing an error and linked my "domino" to the commit with fix in my package.json:

"domino": "https://github.com/ndr/domino.git#87c3ef9d60965ded1e713893c3b7c32a421958b1"

For me it worked (tried with angular 17)

Ive tried your solution, but now Im getting "Error: NotYetImplemented" instead of "Right-hand side of 'instanceof' is not an object"

misha98857 commented 5 months ago

For anyone that runs into this issue but still needs to use Domino, Angular has a forked Domino repository which works with v17s SSR: https://github.com/angular/domino It's not on NPM though, so you will have to install it through git.

I think publishing the fork to npm would be a good idea. Right now, I need to specify the GitHub repository to receive the fixed version. It is not secure and doesn't follow best practices.

Why isn't that a fork published on npm?