GoogleWebComponents / google-apis

Web components for loading Google's JavaScript Libraries
https://elements.polymer-project.org/elements/google-apis
Other
87 stars 64 forks source link

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

Closed livtanong closed 8 years ago

livtanong commented 9 years ago

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

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

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

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

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: https://github.com/GoogleWebComponents/google-apis/issues/22#issuecomment-57492056

ebidel commented 9 years ago

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

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 https://github.com/GoogleWebComponents/google-apis/issues/22#issuecomment-57548938.

ebidel commented 9 years ago

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

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

(merging from #57)

Due to the way the Maps API loads [libraries](https://developers.google.com/maps/documentation/javascript/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 `` 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 `` and so needs a `libraries` property properly set. We leave it up to the end developer to coordinate this (see e.g. https://github.com/GoogleWebComponents/google-streetview-pano/issues/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:

Why the downsides aren't so bad:

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 :)