Fix Angular Universal support #1554
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1554 +/- ##
==========================================
- Coverage 28.99% 28.87% -0.12%
==========================================
Files 32 32
Lines 1452 1458 +6
Branches 197 199 +2
==========================================
Hits 421 421
- Misses 1029 1035 +6
Partials 2 2
Continue to review full report at Codecov.
|
|
Please merge if it works :) |
|
Waiting for this merge as well. Please merge so we can use it with universal |
|
@laurentgoudet thanks for the work. this looks promising. I will review it shortly |
| super(); | ||
| this._config = config || {}; | ||
| this._windowRef = w; | ||
| this._documentRef = d; | ||
| } | ||
|
|
||
| load(): Promise<void> { | ||
| if (!isPlatformBrowser(this.platformId)) { | ||
| // The code is running on the server, skip | ||
| return Promise.resolve(); |
There was a problem hiding this comment.
When someone uses the loader, this would crash on the server side (because the code will assume that the google maps api is loaded) - I think we don't need a change here and a notice in a "running AGM with in universal" apps would be better
There was a problem hiding this comment.
Another note: we need this change in the agm-map component (this would also prevent the loading in most of the use cases)
There was a problem hiding this comment.
Good point, moving the logic to the map component is much cleaner
|
Also: can you remove the added submodule? |
Oops my bad, fixed |
|
I encountered this problem a couple of days ago as well. Hoping that the merge goes well 👍 |
|
Hi, What is the status of this pull request? Thank you! |
|
Hey all, when can we expect this merge to be completed and the issue to be resolved? |
|
Hi All, I am using js-marker-cluster, Its working fine for me, But server side rendering is not working, |
|
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. |
|
Anything we can do to help get this merged in and released? |
ghost
mentioned this pull request
|
How's the progress coming along? |
|
Is this superseded by #1634 or is there still things that are needed for universal support? |
|
@terencehonles No that PR did not fix any Universal related issues. We have been forced to fork this library and patch |
|
@mehrad-rafigh there's been more activity lately so I'd create a new PR if this one is stale and you have a fix |
|
Surely, I will do that. I was just hoping the Author of the PR would update this and get merged :D |
ghost
closed this
Fixes #1052.