Skip to content
This repository has been archived by the owner on Mar 20, 2024. It is now read-only.

Illegal invocation calling window.getComputedStyle #135

Open
2 of 8 tasks
leifmarcus opened this issue Aug 26, 2021 · 29 comments · May be fixed by #151
Open
2 of 8 tasks

Illegal invocation calling window.getComputedStyle #135

leifmarcus opened this issue Aug 26, 2021 · 29 comments · May be fixed by #151

Comments

@leifmarcus
Copy link

I included the PrebotModule in an Angular 12 project and I found that the following line breaks, because the call to getComputedStyle is Illegal.

const gcs = this.getWindow().getComputedStyle;

The getComputedStyle object is set here:

getComputedStyle: window.getComputedStyle,

  • I'm submitting a ...
  • bug report
  • feature request
  • Which parts of preboot are affected by this issue?
  • server side
  • client side
  • inline
  • build process
  • docs
  • tests
  • What is the current behavior?
    Currently it throws an error and breaks the swapping of the root application.

  • If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem by

  • What is the expected behavior?
    Should not fail when running in the browser. I would suggest to add .bind(window) to the declaration.

  • Please tell us about your environment:

  • Browser: [Chrome 92.0.4515.159]
  • Language: [TypeScript 4.3 | ES6/7 | ES5 ]
  • OS: [Mac OS X]

image

@kvirrik
Copy link

kvirrik commented Sep 1, 2021

maybe this is caused by angular v.12.2.x. I have the same problem, after updating angular version

@jakubsobel
Copy link

I have the same problem, after updating from Angular 11.2.5 to 12.2.0.

@boban100janovski
Copy link

boban100janovski commented Sep 3, 2021

Solved it by using this hack.

Install this package "replace-in-file",
Then create a node script to execute before build.

`const replace = require('replace-in-file');
const options = {
files: './node_modules/preboot/fesm2015/preboot.js',
from: 'gcs(serverView).getPropertyValue('display') || 'block';',
to: 'serverView ? (gcs(serverView).getPropertyValue('display') || 'block') : 'block';'
};

try {
const results = replace.sync(options);
console.log('Replacement results:', results);
}
catch (error) {
console.error('Error occurred:', error);
}
`

Then add a new command to package.json
"fix_minify": "node ./node_scripts/fixMinify.js"

Use it before the build
"build:ssr": "npm run fix_minify && ng build --configuration production --localize && ng run URProject:server:production

I think the issue is caused because Angular 12 uses another minifier, that is "Terser", before it was Uglify i think.

@maxisam
Copy link

maxisam commented Sep 8, 2021

It is caused by 12.2.0. It was fine with 12.1.4.

@jsaguet-betclic
Copy link

jsaguet-betclic commented Sep 15, 2021

I encountered the same issue when migrating to 12.2.5.

@CaerusKaru, @alan-agius4 have you seen this issue ?

For information:
No error is present with "@angular-devkit/build-angular": "12.2.0-next.0"
Error occurs starting from "@angular-devkit/build-angular": "12.2.0-next.2"

Tested with all other packages set to 12.2.5

@dragonflypl
Copy link

I have the same issue after migration to latest angular 12.2.x

@rickvandermeij-aanzee
Copy link

Can confirm downgrading back to 12.1.4 solves the issue for me now, waiting for an update

@boban100janovski
Copy link

Seems like "Preboot" is not maintained anymore

@lares83
Copy link

lares83 commented Oct 26, 2021

any news about this issue?

@CaerusKaru
Copy link
Member

Does anyone have a minimal reproduction we can investigate? We're in the process of cleaning up the repo and bringing it up to date, but it's hard to say what the issue is here without a firm reproduction.

@jakubsobel
Copy link

jakubsobel commented Oct 27, 2021

Minimal reproduction: https://github.com/jakubsobel/angular12-preboot-issue
I've created a new angular app using ng new. Then ng add @nguniversal/express-engine and then followed https://github.com/angular/preboot#installation.
Running npm run build:ssr and then npm run serve:ssr is causing Illegal invocation:
Screenshot 2021-10-27 at 13 11 20

@morghim
Copy link

morghim commented Nov 10, 2021

any updates on this ?

@internalsystemerror
Copy link

Having the same issue here on 12.2.x.

@lares83
Copy link

lares83 commented Nov 19, 2021

@CaerusKaru any update about this issue?

@jsaguet
Copy link

jsaguet commented Nov 22, 2021

I submitted a fix in this PR: #146
It seems the error comes from a scope issue.
Something in the build must have changed starting from 12.2

Let's hope it will be checked soon by @CaerusKaru as this error is also blocking for Angular 13 migration

@lares83
Copy link

lares83 commented Dec 17, 2021

do you have any updates about this issue? can @jsaguet 's PR be merged?

@webberig
Copy link

webberig commented Jan 8, 2022

Problem still present in Angular 13

@boban984 's replace script did not solve it, it does not contain the same replacement as PR #146 .

This is an updated version of the script:

fix_minify.js:

const replace = require('replace-in-file');
const options = {
  files: [
    './node_modules/preboot/__ivy_ngcc__/fesm2015/preboot.js',
    './node_modules/preboot/fesm2015/preboot.js',
    './node_modules/preboot/esm2015/api/event.replayer.js',
    './node_modules/preboot/bundles/preboot.umd.js',
    './node_modules/preboot/bundles/preboot.umd.min.js',
  ],
  from: `getComputedStyle: window.getComputedStyle`,
  to: `getComputedStyle: (element, pseudoElt) => window.getComputedStyle(element, pseudoElt)`
};

try {
  const results = replace.sync(options);
  console.log('Replacement results:', results);
}
catch (error) {
  console.error('Error occurred:', error);
}

Took a while. for me to figure out, but after installing this script, you need to clear an Angular build cache:

rm -Rf .angular/cache
node fix_minify.js

After that, it worked for me (confirming #146 will provide the fix)

@Judp0m
Copy link

Judp0m commented Feb 5, 2022

I can confirm that #146 and webberig's comment resolved the issue in my case.
Hopefully it gets officially patched soon, so that the hacky node script can be removed from the codebase.

@lares83
Copy link

lares83 commented Feb 23, 2022

any update about this issue?

@hiepxanh
Copy link

hiepxanh commented Mar 9, 2022

I dont have problem with angular 13, can you test it again? what function do you call?

@hiepxanh
Copy link

hiepxanh commented Mar 9, 2022

ok I see the problem now, basicly, the preboot canot useable sorry to say that

@pavelrazuvalau
Copy link

pavelrazuvalau commented Mar 29, 2022

I removed preboot from my project as I used it only for fixing page flickering. Seems it doesn't actively maintained and instead of applying workarounds I just implemented my own solution for showing overlay. The idea is pretty simple:

app.module.ts

providers: [
    {
      provide: APP_INITIALIZER,
      useFactory: fixPageFlickering,
      deps: [PLATFORM_ID],
    },
  ],
export function fixPageFlickering(platformId: string): () => void {
  return () => {
    if (isPlatformBrowser(platformId)) {
      const transitionStyles = Array.from(document.querySelectorAll('style[ng-transition]'));

      const serverRoot = document.body.querySelector('app-root') as HTMLElement;
      const clientRoot = serverRoot.cloneNode() as HTMLElement;

      serverRoot.setAttribute('ng-non-bindable', '');
      clientRoot.style.display = 'none';

      document.body.insertBefore(clientRoot, serverRoot);

      transitionStyles.forEach((element: HTMLElement) => {
        element.removeAttribute('ng-transition');
      });

      fromEvent(window, 'load').subscribe(() => {
        transitionStyles.forEach((el: HTMLElement) => el.remove());

        clientRoot.style.display = 'block';
        serverRoot.remove();
      });
    }
  };
}

@hiepxanh
Copy link

Good that really nice, i will try it

@rezonant
Copy link

It's unclear how this is related to Angular versions, it is illegal to call getComputedStyle() with this other than window...

> window.getComputedStyle(document.body)
(No error)
> window.getComputedStyle.apply({}, [document.body])
Uncaught TypeError: Illegal invocation

Perhaps this code path wasn't being hit at all before?

In any case, I hope the PR can be merged soon. In the mean time, it is pretty easy to apply the fix and build the package yourself (just edit lib/package.json for your package name, run npm run build, and publish the dist folder). I've done so on @rezonant/[email protected], feel free to use that if you wish.

rezonant added a commit to rezonant/preboot that referenced this issue Apr 20, 2022
hiepxanh added a commit to hiepxanh/preboot that referenced this issue May 6, 2022
@hiepxanh hiepxanh linked a pull request May 6, 2022 that will close this issue
3 tasks
@hiepxanh
Copy link

hiepxanh commented May 6, 2022

@pavelrazuvalau your work is great, I used in my production build and it very smoooooooooooooooooooooooooooooooth

@lares83
Copy link

lares83 commented May 12, 2022

any news about this issue?

@HamzaMoiyadi
Copy link

I removed preboot from my project as I used it only for fixing page flickering. Seems it doesn't actively maintained and instead of applying workarounds I just implemented my own solution for showing overlay. The idea is pretty simple:

app.module.ts

providers: [
    {
      provide: APP_INITIALIZER,
      useFactory: fixPageFlickering,
      deps: [PLATFORM_ID],
    },
  ],
export function fixPageFlickering(platformId: string): () => void {
  return () => {
    if (isPlatformBrowser(platformId)) {
      const transitionStyles = Array.from(document.querySelectorAll('style[ng-transition]'));

      const serverRoot = document.body.querySelector('app-root') as HTMLElement;
      const clientRoot = serverRoot.cloneNode() as HTMLElement;

      serverRoot.setAttribute('ng-non-bindable', '');
      clientRoot.style.display = 'none';

      document.body.insertBefore(clientRoot, serverRoot);

      transitionStyles.forEach((element: HTMLElement) => {
        element.removeAttribute('ng-transition');
      });

      fromEvent(window, 'load').subscribe(() => {
        transitionStyles.forEach((el: HTMLElement) => el.remove());

        clientRoot.style.display = 'block';
        serverRoot.remove();
      });
    }
  };
}

Can you explain the logic behind this? I have tried integrating the same, however post-deployment it fails to work. It works on local serving however.

@hiepxanh
Copy link

hiepxanh commented Sep 7, 2022

it basicly do the same as preboot do:

  • render html with no js
  • angular jump in, remove previous html,
  • after some milisecond, rerender the html, and replace it => this cause flick
    The solution:
  • render html with no js
  • angular jump in, rerender the html
  • after some milisecond, rerender the html, and replace it => this cause flick
    ===> so we not remove it too soon, we just keep it there until angular ready

tested on https://awread.vn using angular 14

@HamzaMoiyadi
Copy link

Beautiful
Thanks @hiepxanh !

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

Successfully merging a pull request may close this issue.