ESModule fix

[josh-hemphill]

Fixes ESModule Error in #317

Changes proposed in this pull request:

• Fixes code substitution so that only the global declaration is removed

[josh-hemphill]

@ndrsn The ESModule doesn't work without this fix, I'd like to get it added.
I didn't test the ES Module bundle correctly in #317, would you happen to have any ideas about how to include tests for ES Modules? I couldn't find a way to add it without breaking the tests.

[ndrsn]

@josh-hemphill apologies, work got a bit busy — I'll take a look at this tomorrow.

I do believe, however, that there must a better way to leave out a conditional than replacing the #NO_GLOBAL? Some kind of variable injection? I'll look into this tomorrow. Thanks for the heads-up!

[ndrsn]

Can we use Rollup's environment values flag instead of the replace? For example (untested):

$ rollup -c --environment EXCLUDE_GLOBAL

and

const excludeGlobal = process && process.env && process.env.EXCLUDE_GLOBAL;

if (window && !window.canvasDatagrid && !window.require && !excludeGlobal) {
  window.canvasDatagrid = function (args) {
    return new Grid(args);
  };
}

[ndrsn]

I didn't test the ES Module bundle correctly in #317, would you happen to have any ideas about how to include tests for ES Modules? I couldn't find a way to add it without breaking the tests.

This is a good point: right now the tests are browser based (Karma), and support for ES modules in browsers isn't complete I believe.

Right now I'd say don't worry about it — if the current Rollup config works, it works. But what we could do is add another, NodeJS-based test runner, to test just the ES module. We need to clean/split up some of the tests any way, so I'll see if I can add this in with that.

[josh-hemphill]

I did some manual tests in the browser with the ES Module to be safe, it all seems to be good.

[ndrsn]

Thanks, @josh-hemphill — I've merged this and the other changes and released it as v0.25.2.