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

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

Open
1 task done
juanmaldonadodev opened this issue Jul 12, 2023 · 36 comments

Comments

@juanmaldonadodev
Copy link

Command

run

Is this a regression?

  • Yes, this behavior used to work in the previous version

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
Copy link
Author

@JeanMeche
Copy link
Member

@juanmaldonadodev
Copy link
Author

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
Copy link

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
Copy link

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
Copy link

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

@ahmadallw
Copy link

any new updated on this issue?

@clydin clydin added the needs: investigation Requires some digging to determine if action is needed label Jul 21, 2023
@deni-rhn
Copy link

deni-rhn commented Jul 27, 2023

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

@Routhinator
Copy link

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
Copy link

manduriomane commented Jul 27, 2023

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

@chakrachi
Copy link

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
Copy link
Contributor

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

@hiepxanh
Copy link
Contributor

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

@alan-agius4
Copy link
Collaborator

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.

@alan-agius4 alan-agius4 added needs: repro steps We cannot reproduce the issue with the information given and removed needs: investigation Requires some digging to determine if action is needed labels Aug 10, 2023
@hiepxanh
Copy link
Contributor

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

@Wycza
Copy link

Wycza commented Aug 10, 2023

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
Copy link
Collaborator

@Wycza, thanks I managed to replicate it.

@alan-agius4 alan-agius4 added type: bug/fix freq1: low Only reported by a handful of users who observe it rarely severity5: regression area: devkit/build-angular devkit/build-angular:server and removed needs: repro steps We cannot reproduce the issue with the information given labels Aug 10, 2023
@chakrachi
Copy link

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

@djgovani
Copy link

djgovani commented Aug 16, 2023

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

This comment was marked as spam.

1 similar comment
@vc-hitesh

This comment was marked as spam.

@REPTILEHAUS
Copy link

Hitting this also v16.2.1

@D-Biela
Copy link

D-Biela commented Sep 11, 2023

Hitting this also v16.2.1

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

@ahmadallw
Copy link

ahmadallw commented Sep 11, 2023

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

@D-Biela
Copy link

D-Biela commented Sep 11, 2023

@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
Copy link

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

@manduriomane
Copy link

Hi to all.
Are there updates?

@ahmadallw
Copy link

Hi all - Any new updates?

@alan-agius4 alan-agius4 self-assigned this Oct 11, 2023
@igordanilovski
Copy link

Same issue for me.

@geferon
Copy link

geferon commented Nov 15, 2023

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
Copy link

+1 facing this issue.

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

@dchicurel
Copy link

dchicurel commented Nov 15, 2023

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
Copy link
Member

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

@ndr
Copy link

ndr commented Dec 17, 2023

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
Copy link

D-Biela commented Dec 29, 2023

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
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests