From e8814a03b1514b2f43bd174e8bfa47a0eaed7cf7 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Mon, 1 May 2017 15:52:03 -0400 Subject: [PATCH 1/4] Better checks for external modules. D3 and HammerJS are intended to be optional modules. Consumers of geojs (at least those that don't use webpack) improperly report that these modules are always available. --- src/d3/d3Renderer.js | 6 +++++- src/mapInteractor.js | 40 +++++++++++++++++++++------------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/d3/d3Renderer.js b/src/d3/d3Renderer.js index 25450c63a0..d02cd4d8ef 100644 --- a/src/d3/d3Renderer.js +++ b/src/d3/d3Renderer.js @@ -635,7 +635,11 @@ registerRenderer('d3', d3Renderer); * @returns {boolean} true if available. */ d3Renderer.supported = function () { - return !!__webpack_modules__[require.resolveWeak('d3')]; // eslint-disable-line + if (!__webpack_modules__[require.resolveWeak('d3')]) { // eslint-disable-line + return false; + } + var d3 = require('d3'); + return d3 !== undefined; }; /** diff --git a/src/mapInteractor.js b/src/mapInteractor.js index 61320c9e82..714d33982a 100644 --- a/src/mapInteractor.js +++ b/src/mapInteractor.js @@ -751,25 +751,27 @@ var mapInteractor = function (args) { (usedInputs.pan || usedInputs.rotate) && __webpack_modules__[require.resolveWeak('hammerjs')]) { // eslint-disable-line var Hammer = require('hammerjs'); - var recog = [], - touchEvents = ['hammer.input']; - if (usedInputs.rotate) { - recog.push([Hammer.Rotate, {enable: true}]); - touchEvents = touchEvents.concat(['rotatestart', 'rotateend', 'rotatemove']); - } - if (usedInputs.pan) { - recog.push([Hammer.Pan, {direction: Hammer.DIRECTION_ALL}]); - touchEvents = touchEvents.concat(['panstart', 'panend', 'panmove']); + if (Hammer !== undefined) { + var recog = [], + touchEvents = ['hammer.input']; + if (usedInputs.rotate) { + recog.push([Hammer.Rotate, {enable: true}]); + touchEvents = touchEvents.concat(['rotatestart', 'rotateend', 'rotatemove']); + } + if (usedInputs.pan) { + recog.push([Hammer.Pan, {direction: Hammer.DIRECTION_ALL}]); + touchEvents = touchEvents.concat(['panstart', 'panend', 'panmove']); + } + var hammerParams = {recognizers: recog, preventDefault: true}; + m_touchHandler = { + manager: new Hammer.Manager($node[0], hammerParams), + touchSupport: m_this.hasTouchSupport(), + lastTime: 0 + }; + touchEvents.forEach(function (touchEvent) { + m_touchHandler.manager.on(touchEvent, m_this._handleTouch); + }); } - var hammerParams = {recognizers: recog, preventDefault: true}; - m_touchHandler = { - manager: new Hammer.Manager($node[0], hammerParams), - touchSupport: m_this.hasTouchSupport(), - lastTime: 0 - }; - touchEvents.forEach(function (touchEvent) { - m_touchHandler.manager.on(touchEvent, m_this._handleTouch); - }); } return m_this; @@ -777,7 +779,7 @@ var mapInteractor = function (args) { //////////////////////////////////////////////////////////////////////////// /** - * Disiconnects events to a map. If the map is not set, then this does + * Disconnects events to a map. If the map is not set, then this does * nothing. * @returns {geo.mapInteractor} */ From fa939ee54195311b7c4c61dc3fa6e7cc9d64606b Mon Sep 17 00:00:00 2001 From: David Manthey Date: Tue, 2 May 2017 14:53:17 -0400 Subject: [PATCH 2/4] Expect hammerjs to be available externally as Hammer. --- webpack.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webpack.config.js b/webpack.config.js index 259828bcd6..f5d6b71577 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -43,7 +43,7 @@ module.exports = { }, externals: { d3: 'd3', - hammerjs: 'hammerjs' + hammerjs: 'Hammer' }, plugins: [ define_plugin, From 89b6801aaf68c2aed6a44ed0f3bb75aab4160cab Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 3 May 2017 12:07:31 -0400 Subject: [PATCH 3/4] Add a test for when Hammer isn't available. --- tests/cases/mapInteractor.js | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/cases/mapInteractor.js b/tests/cases/mapInteractor.js index 0ca066d6b2..1c382a4892 100644 --- a/tests/cases/mapInteractor.js +++ b/tests/cases/mapInteractor.js @@ -1819,3 +1819,38 @@ describe('mapInteractor', function () { expect(map.info.zoom).toBe(2); }); }); +describe('Optional Dependencies', function () { + var $ = require('jquery'); + var geo = require('../test-utils').geo; + + beforeEach(function () { + // create a new div + $('
') + .css({width: '800px', height: '600px'}).appendTo('body'); + }); + + afterEach(function () { + // delete the div and clean up lingering event handlers + $('.testNode').remove(); + $(document).off('.geojs'); + }); + + it('test missing Hammer library', function () { + var old = __webpack_modules__[require.resolveWeak('hammerjs')]; // eslint-disable-line + __webpack_modules__[require.resolveWeak('hammerjs')] = null; // eslint-disable-line + var map = geo.map({node: '#mapNode1'}), + interactor = map.interactor(); + + expect(interactor.hasTouchSupport()).toBe(true); + expect(map.center().x).toBeCloseTo(0); + // We shouldn't process pan touch events + interactor.simulateEvent( + 'panstart', {touch: true, center: {x: 20, y: 20}}); + interactor.simulateEvent( + 'panmove', {touch: true, center: {x: 30, y: 20}}); + interactor.simulateEvent( + 'panend', {touch: true, center: {x: 40, y: 20}}); + expect(map.center().x).toBeCloseTo(0); + __webpack_modules__[require.resolveWeak('hammerjs')] = old; // eslint-disable-line + }); +}); From f925a7a9ae2dcbcd3ac3f5c0b225b764d76614c7 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 3 May 2017 13:16:42 -0400 Subject: [PATCH 4/4] Add a test for when d3 isn't available. --- tests/cases/renderers.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/cases/renderers.js b/tests/cases/renderers.js index a754cf7623..966dbc0169 100644 --- a/tests/cases/renderers.js +++ b/tests/cases/renderers.js @@ -71,6 +71,12 @@ describe('renderers', function () { expect(geo.checkRenderer('d3')).toBe('d3'); + var oldd3 = __webpack_modules__[require.resolveWeak('d3')]; // eslint-disable-line + __webpack_modules__[require.resolveWeak('d3')] = null; // eslint-disable-line + expect(geo.checkRenderer('d3')).toBe(null); + __webpack_modules__[require.resolveWeak('d3')] = oldd3; // eslint-disable-line + expect(geo.checkRenderer('d3')).toBe('d3'); + mockVGLRenderer(false); expect(geo.checkRenderer('vgl')).toBe(null); restoreVGLRenderer();