Angular-google-maps: Clusters are breaking the universal app

[lazarljubenovic]

Issue description

Just importing the cluster module is breaking the app.

Steps to reproduce and a minimal demo of the problem

I have a map where all its markers are under a cluster. The map is wrapped into a conditional so it renders only on client (not on the server) because there are known issues. However, it still breaks the server side rendering because it tries to access window at some point. This probably happens during module initialization, since the component itself never got instantiated.

Current behavior

Apps using cluster cannot be used in a server side app.

Expected/desired behavior

Apps should not break.

angular & angular-google-maps version

• @angular/* at 5.0.0
• @agm/core at 1.0.0-beta.2

Other information

I'm looking for a workaround ATM but, of course, would be nice if users having a Universal app do not have to do anything. We should at least wrap all code with isPlatformBrowser and just list that as a caveat for now, until we figure out how to render maps on the server, if it's even possible.

[SebastianM]

@lazarljubenovic yeah wrapping would be a good solution for now.
cc @jigfox

[jigfox]

I will look into this

[jigfox]

@lazarljubenovic I don't have any experience with universal apps, and I'm not sure how to use isPlatformBrowser properly, would you be able to give me some pointers?

[lazarljubenovic]

@jigfox Sure! You can inject the token PLATFORM_ID into a component, like so:

constructor(@Inject(PLATFORM_ID) private platformId: any) {
}

Then you can use Angular functions such as isPlatformBrowser or isPlatformServer to conditionally run certain pieces of code only on certain platforms.

constructor(@Inject(PLATFORM_ID) private platformId: any) {
  if (isPlatformBrowser(this.platformId)) {
    console.log('This code runs only in browser')
  }
}

[jigfox]

I'm not sure where to put this, the call to window is in code provided by js-marker-clusterer:

markerclusterer.js#L698 and markerclusterer.js#L1270 so cluster-manager.ts#L3 should be wrapped?

Maybe it's necessary to disable the whole cluster plugin on not Browser platforms? But maybe a rewrite markerclusterer.js could help, because it could be rewritten without a window reference

[lazarljubenovic]

Ah, it's code from third-party lib. Jeez. I wonder if just saying something like this would help:

const oldWindow = window || {}
window = {
  setTimeout: oldWindow.setTimeout
}
import 'js-marker-clusterer'
window = oldWindow

But I think that server will break from just finding window while executing code, since the variable has not been instantiated, so that will probably break too, even if it's a good idea.

Could we do the import not at the beginning of the file, but inside a component? What would that even do?

[lazarljubenovic]

By the way, it says on that Google Map repo this:

Please note: This repository is not currently maintained, and is kept for historical purpose only.

So what about we just fork that on our own? Since it's a historical repo, it will never change which means there's no problem just using our own tweaked version which doesn't use window at all?

[lazarljubenovic]

Oh, and regarding this:

But I think that server will break from just finding window while executing code, since the variable has not been instantiated, so that will probably break too, even if it's a good idea.

This looks very basic but I've only learned this a few day ago:

If you try doing if (window == null) or similar and window does not exist as a variable in any visited scope, the code will break with a ReferenceError. However, if you do if (typeof window == 'undefined'), it will work perfectly fine and give expected results even if window has never been declared (which is our issue on server).

I guess I could try playing around and forking our own version of the Cluster repo and see what can be done using this.

[cmddavid]

I tried fixing this using the following approach:

http://ideasintosoftware.com/typescript-conditional-imports/

But I had no luck, as AOT compiler will start complaining the directive can no longer be found.

[lazarljubenovic]

Yeah, AOT compiler statically analyzes the code and thus dynamic imports
sure sound like something that's not possible to do in Angular.

Have you had time trying to fork the repo and just removing access to
window? Looks like it's used only in a couple of places that can easily
be avoided.

On Dec 27, 2017 18:06, "cmddavid" [e-mail] wrote:

I tried fixing this using the following approach:

http://ideasintosoftware.com/typescript-conditional-imports/

But I had no luck, as AOT compiler will start complaining the directive can
no longer be found.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/SebastianM/angular-google-maps/issues/1241#issuecomment-354144018,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHTnkXZ85QbMrFT7timh9MPWDeL5piYiks5tEnkmgaJpZM4QmwTi
.

[cmddavid]

Not yet, I might try it later this week. I'm not a fan of changing the source repo though, even though it is not being maintained at this moment, it might be later. But I guess we don't have any other option. I like the suggestion to completely remove the window references, if that would be possible that would be great.

[lazarljubenovic]

Well, it's been two years since the last update so that's why I think
forking in this particular instance, at least as a temporary solution,
wouldnt be too bad.

On Thu, Dec 28, 2017 at 1:04 AM, cmddavid [e-mail] wrote:

Not yet, I might try it later this week. I'm not a fan of changing the
source repo though, even though it is not being maintained at this moment,
it might be later. But I guess we don't have any other option. I like the
suggestion to completely remove the window references, if that would be
possible that would be great.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/SebastianM/angular-google-maps/issues/1241#issuecomment-354198599,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHTnkfZdUcmrInBzlNL41wJ6znRzWVCoks5tEtsHgaJpZM4QmwTi
.

[mbrezovsky]

I forked js-marker-clusterer project and made the fix. It prevents initialization MarkerClusterer when window is not defined (on server side rendering). So marker clusterer works correctly in both cases now.

It's published in npm. https://www.npmjs.com/package/js-marker-clusterer-universal

js-marker-clusterer dependency should be replaced by js-marker-clusterer-universal to take effect.

[cmddavid]

@mbrezovsky nice work, it is working well for me. I did get one error related to @agm/js-marker-clusterer using imports, something that is not allowed yet on the Node.js/Firebase side, so I had to convert the code to ES2015 using Anthony Nahas his solution proposed here: https://github.com/SebastianM/angular-google-maps/issues/1052#issuecomment-331150772

[jonaspraem]

Any updates on this issue?

[cmddavid]

@AoschkA I'm still using the workaround as mentioned in my earlier comment. You can find my example here: https://github.com/cmddavid/js-marker-clusterer.git

[jonaspraem]

@cmddavid Your readme just explains how to install agm/js-marker-clusterer. How does one integrate this into our own projects?

[cmddavid]

@AoschkA, the repo is just the compiled output, I did not any instructions, you can add it to your package.json like this:

"@agm/js-marker-clusterer": "git+https://github.com/cmddavid/js-marker-clusterer.git"

When you install it, you will have the js-marker-clusterer without window is undefined errors, and compiled to ES2015 so it's compatible with services like Firebase.

[jonaspraem]

Thx that works. Would be nice if this could be implemented in AGM though. But I appreciate the solution @cmddavid !

[maksimbykov]

@cmddavid I made like you said, but there is error ERROR in ./node_modules/@agm/js-marker-clusterer/services/managers/cluster-manager.js
Module not found: Error: Can't resolve 'js-marker-clusterer-universal' in '.\node_modules\@agmjs-marker-clusterer\services\managers'

[cmddavid]

@maksimbykov can you confirm js-marker-clusterer-universal is in your node_modules folder? perhaps it did not fully install, just in case you could run npm i --save js-marker-clusterer-universal. Also something weird seems to be going on with the file paths there. The first one has a double slash // and the second one misses a backslash \ between node_modules and @agm.

Also worth mentioning I did not check if the package still works with angular 6.

[maksimbykov]

@cmddavid thank you very much! probably yesterday I was tired and inattentive

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[newprogrammer1991]

"@agm/js-marker-clusterer": "git+https://github.com/cmddavid/js-marker-clusterer.git" sorry but it doesn't work(don't install anything)

[nrozic]

I forked js-marker-clusterer project and made the fix. It prevents initialization MarkerClusterer when window is not defined (on server side rendering). So marker clusterer works correctly in both cases now.

It's published in npm. https://www.npmjs.com/package/js-marker-clusterer-universal

js-marker-clusterer dependency should be replaced by js-marker-clusterer-universal to take effect.

Thanks a lot man, you just saved me... kudos :)

[alexnoise79]

i still have this issue, why is it marked as closed??

[lazarljubenovic]

You literally downvoted the explanation on why it was closed. You've seen why it was closed.

[ghost]

@lazarljubenovic we are going to move to https://github.com/googlemaps/v3-utility-library/tree/master/packages/markerclustererplus anyway

[alexnoise79]

You literally downvoted the explanation on why it was closed. You've seen why it was closed.

so now we can reopen it :dancer:

anyway is not simpler to import js-marker-clusterer inside the library and create an exportable module?

i did this on mine:

https://github.com/alexnoise79/js-marker-clusterer (see src/markerclusterer.js)

and then on:
node_modules/@agm/js-marker-clusterer/fesm2015/agm-js-marker-clusterer.js

i replaced declare Markerclusterer with:

import * as MarkerClusterer from 'js-marker-clusterer';

it works and compile ssr 100%

[IgorKurkov]

My solution base on @mbrezovsky comment above - so

• install his npm package https://www.npmjs.com/package/js-marker-clusterer-universal (or make own fork...)
• make few commands in package.json

"install:clusterer-universal": "npm i js-marker-clusterer-universal && rm -rf node_modules/js-marker-clusterer && mv node_modules/js-marker-clusterer-universal node_modules/js-marker-clusterer",
"postbuild": "npm run install:clusterer-universal"

and it will be installed after all and replaced.
And SSR works! :)