uiuniversal / ngu-carousel

Angular Universal carousel
https://ngu-carousel.netlify.app
MIT License
329 stars 104 forks source link

Universal regression: ReferenceError: window is not defined #202

Closed Fabyao closed 4 years ago

Fabyao commented 4 years ago

Thank you for the great carousel. I have recently upgraded to version 2.0.0 which fixes the compatibility issue with angular 9. However the carousel no longer works in server side.

Its throws an error: ReferenceError: window is not defined

I have reverted back to ngu carousel 1.5.5 which works fine in server side but fails once loaded as I am using angular 9.

sheikalthaf commented 4 years ago

@Fabyao Can please post the error log so that we can fix the issue.

Fabyao commented 4 years ago

@sheikalthaf Thanks for your response. These are the steps to reproduce.

Given I have an Angular 9 project And It is setup for Universal app (SSR) When I install NGU Carousel version 2.0.0 Then The application fails with the following error

Error log })(window, document, 'Hammer'); ^ ReferenceError: window is not defined

Let me know if you need more details

sheikalthaf commented 4 years ago

@santoshyadav198613 do you have any idea why this happens in latest version?

santoshyadavdev commented 4 years ago

Hi, we should not be using window object if we need to support SSR, will fix this over this weekend.

sheikalthaf commented 4 years ago

But we are checking isPlatforomBrowser before using window

santoshyadavdev commented 4 years ago

But we are checking isPlatforomBrowser before using window

Yes just saw the code we are already handling the code. @Fabyao can you create a repo that produces the same error.

santoshyadavdev commented 4 years ago

I was able to reproduce it, and there is an related issue with some work around https://github.com/hammerjs/hammer.js/issues/930 will try to fix.

Fabyao commented 4 years ago

@santoshyadav198613 thanks for pointing us in the right direction. I have fixed the issue by doing: npm install null-loader --save-dev

and adding this to my webpack server config: { test: /hammerjs/, loader: "null-loader" }

santoshyadavdev commented 4 years ago

Glad it worked for you, in think we should add it somewhere.

sheikalthaf commented 4 years ago

@santoshyadav198613 yes. So that it will be help to add use this library in SSR with ease

kumargauravhcl commented 4 years ago

@Fabyao - Can you pleae copy paste whole file? I am not using webpack for angular 9 project and curious how we can do it?

ggarg2 commented 4 years ago

@sheikalthaf @santoshyadav198613 Can we use HammerModule which is mention in the Ivy compatibility guide ( https://angular.io/guide/ivy-compatibility#less-common-changes ). HammerModule is a part of @angular/platform-browser. Maybe we will not see HammerJs window not defined error.

santoshyadavdev commented 4 years ago

I did try that, the problem is HammerModule uses window object, will check what can be the perfect fix for this.

ggarg2 commented 4 years ago

Thanks @santoshyadav198613

Khokhlachov commented 4 years ago

@sheikalthaf can you provide some ETA? This issue is block using your library on angular 9 with ssr.

santoshyadavdev commented 4 years ago

@sheikalthaf can you provide some ETA? This issue is block using your library on angular 9 with ssr.

Can you refer comment from @Fabyao and see if it works for you.

helgetan commented 4 years ago

i have the same problem, the workaround with webpack null-loader doesn't work for me

Angular 9.1.0 ngu 3.0.o

})(window, document, 'Hammer');
   ^

ReferenceError: window is not defined
    at Object.../../node_modules/hammerjs/hammer.js (/../dist/website-server.js:288141:4)
    at __webpack_require__ (/../dist/website-server.js:167:30)
    at Module.../../node_modules/@ngu/carousel/__ivy_ngcc__/fesm2015/ngu-carousel.js (/../dist/website-server.js:248971:66)
Khokhlachov commented 4 years ago

@sheikalthaf can you provide some ETA? This issue is block using your library on angular 9 with ssr.

Can you refer comment from @Fabyao and see if it works for you.

@santoshyadav198613 Of Course it is not solution for regular angular 9 as it based on webpack loader -> regular angular 9 SSR enviroment do not use webpack config anymore

helgetan commented 4 years ago

A workaround which works for me is using instead of the angular builder the customWebpackBuilder, which is a wrapper around the standard angular9 builder. This way you don't have to configure a complete webpack config, just use the parts that you need ontop of the angular build-process

npm i null-loader @angular-builders/custom-webpack

and the following config in your angular.json

  "server": {
          "builder": "@angular-builders/custom-webpack:server",
          "options": {
            "customWebpackConfig": {
              "path": "webpack.extra.config.js",
              "mergeStrategies": {
                "plugins": "append"
              }
            },
....
}

webpack.extra.config.js

const webpack = require('webpack');

module.exports = {
    module: {
        rules: [
            {
                test: /hammerjs/,
                loader: 'null-loader'
            }
        ]
    },
};
santoshyadavdev commented 4 years ago

Thanks @helgetan, I also have another workaround, I will try it on weekend.

Fabyao commented 4 years ago

@santoshyadav198613 @Khokhlachov Sorry for the late reply its been a busy few weeks. The project I work on needs to meet certain requirements that are otherwise impossible to achieve with the angular CLI. So we use webpack directly. This gives us more control over what webpack outputs. The workaround to get ngu-carousel to work with Angular 9 universal is very much the same as @helgetan without the custom webpack builder

santoshyadavdev commented 4 years ago

@sheikalthaf can you provide some ETA? This issue is block using your library on angular 9 with ssr.

Can you refer comment from @Fabyao and see if it works for you.

@santoshyadav198613 Of Course it is not solution for regular angular 9 as it based on webpack loader -> regular angular 9 SSR enviroment do not use webpack config anymore

Hi @Khokhlachov , Can you do me a favor and try the code from https://github.com/uiuniversal/ngu-carousel/tree/feat/fix-hammerjs-with-ssr this branch and check if this works for you.

santoshyadavdev commented 4 years ago

Hi Everyone, Some good news, finally i was able to make it work with both SSR and SPA, for SSR you can refer to https://github.com/santoshyadav198613/ngu-carousel-ssr-demo

Use version 2.0.3

santoshyadavdev commented 4 years ago

Hi @Khokhlachov , Can you try 2.0.3 you can also refer to https://github.com/uiuniversal/ngu-carousel/tree/feat/fix-hammerjs-with-ssr

santoshyadavdev commented 4 years ago

Closing this reopen if you face any issue.