Correct `crossfilter2` dependency for AMD usage.

shea-parkes commented 5 years ago

The current ~UMD build doesn't correctly specify the crossfilter2 dependency for AMD style dependencies.

In particular, this is on the first line of the ~compiled reductio.js:

if("function"==typeof define&&define.amd)define([],e)

And then it later tries to load crossfilter via:

var crossfilter = (typeof window !== "undefined" ? window['crossfilter'] : typeof global !== "undefined" ? global['crossfilter'] : null);

I believe the dependency should be properly expressed via something like this:

if("function"==typeof define&&define.amd)define(['crossfilter2'],e)

I'm sure this is all supposed to be handled by the build tools, but their current settings are causing me to have to export crossfilter2 as a global to use in an AMD environment.

For what it's worth, dc.js works well in an AMD environment. Their ~compiled form includes this definition:

if(typeof define === "function" && define.amd) {
        define(["d3", "crossfilter2"], _dc);
    }

I realize this may not be a high priority for you, but I can always hope that it's an easy fix. Or at least I can warn other users (that might still be using an AMD environment).

Thanks for the nice tool. It's working very nicely in a 2-level aggregation scenario with a dc.js solution.

esjewett commented 5 years ago

@shea-parkes Any chance you have a sample project that I can run some tests on? My guess is that the browserify-shim configuration is to blame, but I'm not sure.

shea-parkes commented 5 years ago

Thanks for looking into this sir, here's a reproducible example, built from the dc Scatter Brushing example: https://dc-js.github.io/dc.js/examples/scatter-brushing.html

First, the example converted to work with require.js (sans reductio) (notice I went ahead and double defined crossfilter as crossfilter and crossfilter2 just to be defensive against the name swap):

<!DOCTYPE html>
<html lang="en">
<head>
  <title>dc.js - Scatter Plot Brushing Example</title>
    <meta charset="UTF-8">
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/dc/3.0.11/dc.min.css" />
</head>
<body>
<div class="container">
  <p>Brush on one chart to see the points filtered on the other.</p>
  <div id="test1"></div>
  <div id="test2"></div>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js"></script>
  <script>
    requirejs.config({
      paths: {
        d3: 'https://cdnjs.cloudflare.com/ajax/libs/d3/5.9.1/d3.min',
        crossfilter2: 'https://cdnjs.cloudflare.com/ajax/libs/crossfilter2/1.4.6/crossfilter.min',
        crossfilter: 'https://cdnjs.cloudflare.com/ajax/libs/crossfilter2/1.4.6/crossfilter.min',
        dc: 'https://cdnjs.cloudflare.com/ajax/libs/dc/3.0.11/dc.min',
      }
    });
  </script>
  <script type="text/javascript">
    require(['d3', 'crossfilter2', 'dc'], (d3, crossfilter, dc) => {
      var chart1 = dc.scatterPlot("#test1");
      var chart2 = dc.scatterPlot("#test2");
      var data = "x,y,z\n" +
          "1,1,3\n" +
          "5,2,11\n" +
          "13,13,13\n"+
          "5,3,20\n"+
          "12,12,10\n"+
          "3,6,8\n"+
          "15,2,9\n"+
          "8,6,14\n"+
          "1,4,9\n"+
          "8,8,12\n";
      var data = d3.csvParse(data);

      data.forEach(function (x) {
          x.x = +x.x;
          x.y = +x.y;
          x.z = +x.z;
      });

      var ndx = crossfilter(data),
          dim1 = ndx.dimension(function (d) {
              return [+d.x, +d.y];
          }),
          dim2 = ndx.dimension(function (d) {
              return [+d.y, +d.z];
          }),
          group1 = dim1.group(),
          group2 = dim2.group();

      chart1.width(300)
          .height(300)
          .x(d3.scaleLinear().domain([0, 20]))
          .yAxisLabel("y")
          .xAxisLabel("x")
          .clipPadding(10)
          .dimension(dim1)
          .excludedOpacity(0.5)
          .group(group1);

      chart2.width(300)
          .height(300)
          .x(d3.scaleLinear().domain([0, 20]))
          .yAxisLabel("z")
          .xAxisLabel("y")
          .clipPadding(10)
          .dimension(dim2)
          .excludedColor('#ddd')
          .group(group2);

      dc.renderAll();

    })

  </script>

</div>
</body>
</html>

And now here's trying to bring in reductio and use some crossfilter functionality:

<!DOCTYPE html>
<html lang="en">
<head>
  <title>dc.js - Scatter Plot Brushing Example</title>
    <meta charset="UTF-8">
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/dc/3.0.11/dc.min.css" />
</head>
<body>
<div class="container">
  <p>Brush on one chart to see the points filtered on the other.</p>
  <div id="test1"></div>
  <div id="test2"></div>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js"></script>
  <script>
    requirejs.config({
      paths: {
        d3: 'https://cdnjs.cloudflare.com/ajax/libs/d3/5.9.1/d3.min',
        crossfilter2: 'https://cdnjs.cloudflare.com/ajax/libs/crossfilter2/1.4.6/crossfilter.min',
        crossfilter: 'https://cdnjs.cloudflare.com/ajax/libs/crossfilter2/1.4.6/crossfilter.min',
        dc: 'https://cdnjs.cloudflare.com/ajax/libs/dc/3.0.11/dc.min',
        reductio: 'https://cdnjs.cloudflare.com/ajax/libs/reductio/0.6.3/reductio.min',
      }
    });
  </script>
  <script type="text/javascript">
    require(['d3', 'crossfilter2', 'dc', 'reductio'], (d3, crossfilter, dc, reductio) => {
      var chart1 = dc.scatterPlot("#test1");
      var chart2 = dc.scatterPlot("#test2");
      var data = "x,y,z\n" +
          "1,1,3\n" +
          "5,2,11\n" +
          "13,13,13\n"+
          "5,3,20\n"+
          "12,12,10\n"+
          "3,6,8\n"+
          "15,2,9\n"+
          "8,6,14\n"+
          "1,4,9\n"+
          "8,8,12\n";
      var data = d3.csvParse(data);

      data.forEach(function (x) {
          x.x = +x.x;
          x.y = +x.y;
          x.z = +x.z;
      });

      var ndx = crossfilter(data),
          dim1 = ndx.dimension(function (d) {
              return [+d.x, +d.y];
          }),
          dim2 = ndx.dimension(function (d) {
              return [+d.y, +d.z];
          }),
          group1 = dim1.group(),
          group2 = dim2.group();

      var reducer = reductio().valueList(x => x.x)
      reducer(group1)

      chart1.width(300)
          .height(300)
          .x(d3.scaleLinear().domain([0, 20]))
          .yAxisLabel("y")
          .xAxisLabel("x")
          .clipPadding(10)
          .dimension(dim1)
          .excludedOpacity(0.5)
          .group(group1);

      chart2.width(300)
          .height(300)
          .x(d3.scaleLinear().domain([0, 20]))
          .yAxisLabel("z")
          .xAxisLabel("y")
          .clipPadding(10)
          .dimension(dim2)
          .excludedColor('#ddd')
          .group(group2);

      dc.renderAll();

    })

  </script>

</div>
</body>
</html>

The latter document should produce this error:

Uncaught TypeError: Cannot read property 'bisect' of undefined
    at Object.add (reductio.min.js:1)
    at Object.r [as build] (reductio.min.js:1)
    at e (reductio.min.js:1)
    at require (dc_scatter_require_fail.html:59)
    at Object.execCb (require.min.js:1)
    at e.check (require.min.js:1)
    at e.<anonymous> (require.min.js:1)
    at require.min.js:1
    at require.min.js:1
    at each (require.min.js:1)

shea-parkes commented 5 years ago

Oh, and my current workaround if someone is looking for a way to make this work:

define('globalCrossfilter', ['crossfilter2'], crossfilter => window.crossfilter = crossfilter)

And then be sure to require globalCrossfilter. The current ~compiled reductio will use a ~global crossfilter if available.

crossfilter / reductio

Correct `crossfilter2` dependency for AMD usage. #57