Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different instances of google-maps-api with different specified libraries causes conflicts #22

Closed
livtanong opened this issue Sep 30, 2014 · 9 comments · Fixed by #58
Closed

Comments

@livtanong
Copy link

Somewhat related to #20.

Especially problematic with <google-map>, which has its own import of <google-maps-api> with hard-coded libraries="places". If on the same page you have a <google-maps-api> with libraries="places,geometry", two things happen:

  1. We get an error: You have included the Google Maps API multiple times on this page. This may cause unexpected errors. This suggests that core-shared-library treats these two as separate, which causes the google warning.
  2. When trying to use methods from one instance of the API on the other (like the map) things get weird.
    For example, when trying to render a polyline from an encoded string (requires the geometry library) onto a <google-map map="{{gmap}}">, gmap is not considered by the geometry library as type Map.
    However, a map created manually without the <google-map> (and therefore doesn't use its own <google-maps-api>) works just fine.
    If I change the google-maps-api.html default value for libraries to "places,geometry", the geometry library works properly when used with google-map.
@ebidel
Copy link
Contributor

ebidel commented Sep 30, 2014

If you're setting libraries anywhere, you need to use the same value across all google-map-api or google-map related tags.

<google-maps libraries="geometry,places"></google-maps>
<google-maps-api libraries="geometry,places"></google-maps-api>

https://github.com/GoogleWebComponents/google-map/blob/master/google-map-directions.html#L90

@livtanong
Copy link
Author

Oh, you guys added the libraries attribute! (i just changed my bower.json entries for google-map and google-api to #master) That wasn't there last time :P Thanks for the tip!

Having to maintain the same value for all libraries across google maps related elements isn't very maintainable though. Is there a DRYer implementation planned in the future? (While I can think of ways to parametrize this, so far everything's a bit roundabout)

@ebidel
Copy link
Contributor

ebidel commented Oct 1, 2014

Yea, the current setup isn't ideal. We knew this would happen by supporting the libraries parameter. Most people will only load <google-map> and set it. Some will use multiple maps elements and need to set the same libraries on both elements. Where it gets tricky is other 3rd party elements that use <google-maps-api> and do not use the same libraries or elements that are instantiated later and request different maps api loading endpoints. Those will produce the double maps load error.

One solution is to load remove the libraries param and always load all libraries.

@livtanong
Copy link
Author

Do you think it's possible to extend core-shared-library to include the ability to get the union of all the arguments of all instances of a shared library?—
Levi

On Thu, Oct 2, 2014 at 12:17 AM, Eric Bidelman notifications@github.com
wrote:

Yea, the current setup isn't ideal. We knew this would happen by supporting the libraries parameter. Most people will only load <google-map> and set it. Some will use multiple maps elements and need to set the same libraries on both elements. Where it gets tricky is other 3rd party elements that use <google-maps-api> and do not use the same libraries or elements that are instantiated later and request different maps api loading endpoints. Those will produce the double maps load error.

One solution is to load remove the libraries param and always load all libraries.

Reply to this email directly or view it on GitHub:
#22 (comment)

@ebidel
Copy link
Contributor

ebidel commented Oct 1, 2014

What happens when you load a new component later on that requests an additional library? That'll look like a different request, core-shared-library will load the Maps API, and you'll get an error about double loading the library.

@livtanong
Copy link
Author

Perhaps it could check the API being requested by the new component, and see that that specific API is already loaded. It would then perform the union of arguments, then replace the existing loaded API. Would that work?

On Oct 2, 2014, at 06:10, Eric Bidelman notifications@github.com wrote:

What happens when you load a new component later on that requests an additional library? That'll look like a different request, core-shared-library will load the Maps API, and you'll get an error about double loading the library.


Reply to this email directly or view it on GitHub #22 (comment).

@ebidel
Copy link
Contributor

ebidel commented Oct 2, 2014

This is more or less how core-shared-lib works today, but it caches an API by its full URL. The problem is that it doesn't work for an API like the Maps API that you can load additional libraries via a parameter.

Component 1 loads: https://maps.googleapis.com/maps/api/js?libraries=visualization,places
(later) Component 2 loads: https://maps.googleapis.com/maps/api/js?libraries=visualization,geometry

When component 2 loads, the maps api is already loaded and gives you the error. See http://jsbin.com/zuzata/1/edit.

To be 100% honest, the console warning may just be a warning. This may all be moot.

cc @brendankenny

@livtanong
Copy link
Author

What I mean to say is, is it possible to get the union of visualization, places, and geometry?

The console warning is not just a warning though. A more concrete example:

comp1 gets from https://maps.googleapis.com/maps/api/js?libraries=places
comp2 gets from https://maps.googleapis.com/maps/api/js?libraries=places,geometry

map = new comp1.Map();

map instanceof comp1.Map // true
map instanceof comp2.Map // false

@brendankenny
Copy link
Member

(merging from #57)

Due to the way the Maps API loads libraries (designed in the ancient days of ~2008) you have to load all the libraries you'll need on first load and can't load more libraries later. You also need to make sure the API is only loaded once on each page.

The second problem is taken care of by <google-maps-api> but the first means that every other element that uses it needs to specify the same set of libraries as the rest for them to work on the same page together. This can be problematic if you're trying to make a SPA and have a need for different libraries in different parts of your app, or if you don't even realize an element you're using contains a <google-maps-api> and so needs a libraries property properly set. We leave it up to the end developer to coordinate this (see e.g. GoogleWebComponents/google-streetview-pano#15). Once you know the problem exists, it's easy to diagnose and fix, but it's annoying.

To eliminate these problems, I propose we always load all four libraries whenever the Maps API is loaded and eliminate the libraries property on all google-map-* elements. Configuration gets simpler, you don't have to repeat yourself over and over to use the exact same libraries you already loaded, and we eliminate a class of bugs. Boom.

Downsides:

  • libraries are loaded as separate JS files, requiring four more network requests dependent on the bootstrap script loading and executing
  • some amount of parse and initialization time for the libraries, even if unused (beyond what the JS engine has to do because of the way the Google Maps loader works)

Why the downsides aren't so bad:

  • total download size of the four libraries is ~5.5KB with gzip (~9.5KB after unzip)
  • there are already 15 scripts loaded with a vanilla Maps API load that we weren't worrying about before :)
  • the library script downloads are launched immediately by the bootstrap script, in parallel to the main.js script file
  • Maps API files are served with QUIC or HTTP2 or SPDY where available, so extra requests are multiplexed over an already open connection

Any possible network performance problem should be fixed at the browser/Maps API/serving infrastructure levels anyways. We shouldn't burden all the google-map-* components with this additional API complexity.

As Eric notes above, the suggestion in this thread will not work because of the temporal aspect of the problem: the only union of libraries that will include all possible libraries needed by elements that could be added to the page in the future is the set of all libraries. Might as well load all of them :)

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

Successfully merging a pull request may close this issue.

3 participants