angular / preboot

Coordinate transfer of state from server to client view for isomorphic/universal JavaScript web applications
MIT License
382 stars 51 forks source link

Illegal invocation calling window.getComputedStyle #135

Open leifmarcus opened 2 years ago

leifmarcus commented 2 years ago

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

https://github.com/angular/preboot/blob/022ce61601772f7df794e93c740f8d54664f0a0a/src/lib/api/event.replayer.ts#L146

The getComputedStyle object is set here: https://github.com/angular/preboot/blob/022ce61601772f7df794e93c740f8d54664f0a0a/src/lib/api/event.replayer.ts#L21

image

kvirrik commented 2 years ago

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

jakubsobel commented 2 years ago

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

boban100janovski commented 2 years ago

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 commented 2 years ago

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

jsaguet-betclic commented 2 years ago

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 commented 2 years ago

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

rickvandermeij-aanzee commented 2 years ago

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

boban100janovski commented 2 years ago

Seems like "Preboot" is not maintained anymore

lares83 commented 2 years ago

any news about this issue?

CaerusKaru commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

any updates on this ?

internalsystemerror commented 2 years ago

Having the same issue here on 12.2.x.

lares83 commented 2 years ago

@CaerusKaru any update about this issue?

jsaguet commented 2 years ago

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 commented 2 years ago

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

webberig commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

any update about this issue?

hiepxanh commented 2 years ago

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

hiepxanh commented 2 years ago

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

pavelrazuvalau commented 2 years ago

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 commented 2 years ago

Good that really nice, i will try it

rezonant commented 2 years ago

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/preboot@8.0.0, feel free to use that if you wish.

hiepxanh commented 2 years ago

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

lares83 commented 2 years ago

any news about this issue?

HamzaMoiyadi commented 1 year ago

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 commented 1 year ago

it basicly do the same as preboot do:

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

HamzaMoiyadi commented 1 year ago

Beautiful Thanks @hiepxanh !