search

google / google-visualization-issues

288 stars 35 forks source link

ChartWrapper won't automatically load Map packages #2903

Open AlmaniaM opened 3 years ago

AlmaniaM commented 3 years ago

Issue

According to the ChartWrapper docs, the ChartWrapper handles loading all necessary libraries. If you have third-party libraries then you must load them yourself explicitly.

I'm having problems loading a 'Map' chart type. When trying to load a map I receive the error 'object is not iterable (cannot read property Symbol(Symbol.iterator))' If I don't explicitly load the 'map' package. Here's an example piece of code that can be run to reproduce the issue.

<html>
  <head>
    <script type="text/javascript" src="https://www.gstatic.com/charts/loader.js"></script>
    <script type="text/javascript">
      google.charts.load('current');
      // google.charts.load('current', { packages: ['map']});
      google.charts.setOnLoadCallback(drawMap);

      function drawMap() {
        let mapWrapper = new google.visualization.ChartWrapper({
            chartType: 'Map',
            dataTable: [
              ['lattitude', 'longitude', 'label'],
              [-37.7647, 144.9393, 'hello'],
              [40.4165, -3.7026, 'hello2']
            ],
            options: {'title': 'Stuff'},
            container: document.getElementById('map_div')
        });

        mapWrapper.draw();
      }
    </script>
  </head>
  <body style="font-family: Arial;border: 0 none;">
    <div id="map_div" style="width: 600px; height: 400px;"></div>
  </body>
</html>

If you run as-is, you will receive the error. If you uncomment the load call with the 'map' package then everything loads just fine. Is the Google Maps library considered a third-party library or is this a bug or am I missing something? Any help here would be greatly appreciated.

After debugging

Looks like it might be a bug in the ChartWrapper? I did some debugging and found a possible issue where ChartWrapper (I think) checks for the chart type by looking at possible namespaces. The issue is that when there aren't any loaded packages that contain a 'Map' constructor function it lands on just 'Map'. The check always starts from the Window object so when all other namespaces fail it succeeds on Window.Map, which is just a JavaScript Map data structure. I tested removing the last part of the array (the 'Map' value) and it successfully loaded the Google Maps api.

See image for more detail google-chart-wrapper-debug

dlaliberte commented 3 years ago

Hi Almania,

It does look like a bug, but it is more of a special case. If you don't load the Map chart specifically with the google.charts.load packages option, then it does look for the class name to see if it is already defined, and uses what it finds if so. So it ends up finding the Map data structure class. If it were to find nothing, then it would look up how to load the Map visualization. This is how the ChartWrapper has worked for many years, and I suspect there are many uses of visualization classes that are defined by users, such that we cannot change this behavior now.

AlmaniaM commented 3 years ago

@dlaliberte Should the Google Charts documentation reflect this special case then? The documentation promises that the ChartWrapper will handle loading all google.visualization libraries for you. It says you must load third-party libraries explicitly but google.visualization.Map is still part of the visualization library isn't it?

dlaliberte commented 3 years ago

Yes, the documentation should be updated to point out this conflict. It is JavaScript that changed, by the way, since Map was not previously a built-in class. It is not very likely that any of the other chart names will have this conflict, and going forward, the Map chart will likely be replaced by something more general, with a better name.

AlmaniaM commented 3 years ago

@dlaliberte Ah, that makes sense. Thank you for the clarification!