fix(GoogleMapsLoader): inline the import to scriptjs (#1427)

* fix(GoogleMapsLoader): inline require * refactor(GoogleMapsLoader): add .jsdom to the test file * chore(babel): add dynamic-import-node * fix(GoogleMapsLoader): use import rather than require * chore(maps): avoid to transpile dyanmic import with Rollup * style(GoogleMapsLoader): use destructuring for dynamic import
algolia · Jul 11, 2018 · 8019416 · 8019416
1 parent 1b416ca
commit 8019416
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 37 deletions.
diff --git a/packages/react-instantsearch-dom-maps/.babelrc b/packages/react-instantsearch-dom-maps/.babelrc
@@ -17,7 +17,8 @@
         "stage-2"
       ],
       "plugins": [
-        "lodash"
+        "lodash",
+        "dynamic-import-node"
       ]
     },
     "test": {
@@ -37,7 +38,8 @@
         "stage-2"
       ],
       "plugins": [
-        "lodash"
+        "lodash",
+        "dynamic-import-node"
       ]
     },
     "production": {
@@ -57,10 +59,33 @@
         "stage-2"
       ],
       "plugins": [
-        "lodash"
+        "lodash",
+        "dynamic-import-node"
       ]
     },
     "es": {
+      "presets": [
+        [
+          "env",
+          {
+            "targets": {
+              "browsers": [
+                "last 2 versions",
+                "ie >= 9"
+              ]
+            },
+            "modules": false
+          }
+        ],
+        "react",
+        "stage-2"
+      ],
+      "plugins": [
+        "lodash",
+        "dynamic-import-node"
+      ]
+    },
+    "rollup": {
       "presets": [
         [
           "env",

diff --git a/packages/react-instantsearch-dom-maps/package.json b/packages/react-instantsearch-dom-maps/package.json
@@ -36,7 +36,7 @@
     "build": "yarn build:cjs && yarn build:es && yarn build:umd",
     "build:cjs": "babel src --out-dir dist/cjs --ignore __tests__,__mocks__ --quiet",
     "build:es": "BABEL_ENV=es babel src --out-dir dist/es --ignore __tests__,__mocks__ --quiet",
-    "build:umd": "BABEL_ENV=es rollup -c rollup.config.js",
+    "build:umd": "BABEL_ENV=rollup rollup -c rollup.config.js",
     "test": "jest",
     "release": "yarn clean && yarn build && yarn publish --new-version $VERSION",
     "release:beta": "yarn clean && yarn build && yarn publish --tag beta --new-version $VERSION"
@@ -50,6 +50,7 @@
   "devDependencies": {
     "babel-cli": "6.26.0",
     "babel-core": "6.26.0",
+    "babel-plugin-dynamic-import-node": "2.0.0",
     "babel-plugin-external-helpers": "6.22.0",
     "babel-plugin-lodash": "3.3.4",
     "babel-preset-env": "1.7.0",

diff --git a/packages/react-instantsearch-dom-maps/src/GoogleMapsLoader.js b/packages/react-instantsearch-dom-maps/src/GoogleMapsLoader.js
@@ -1,6 +1,5 @@
 import { Component } from 'react';
 import PropTypes from 'prop-types';
-import injectScript from 'scriptjs';
 
 class GoogleMapsLoader extends Component {
   static propTypes = {
@@ -20,16 +19,21 @@ class GoogleMapsLoader extends Component {
   isUnmounting = false;
 
   componentDidMount() {
-    const { apiKey, endpoint } = this.props;
-    const operator = endpoint.indexOf('?') !== -1 ? '&' : '?';
-    const endpointWithCredentials = `${endpoint}${operator}key=${apiKey}`;
-
-    injectScript(endpointWithCredentials, () => {
-      if (!this.isUnmounting) {
-        this.setState(() => ({
-          google: window.google,
-        }));
-      }
+    // Inline the import to avoid to run the module on the server (rely on `document`)
+    // Under the hood we use `dynamic-import-node` to transpile the `import` to `require`
+    // see: https://github.com/algolia/react-instantsearch/issues/1425
+    return import('scriptjs').then(({ default: injectScript }) => {
+      const { apiKey, endpoint } = this.props;
+      const operator = endpoint.indexOf('?') !== -1 ? '&' : '?';
+      const endpointWithCredentials = `${endpoint}${operator}key=${apiKey}`;
+
+      injectScript(endpointWithCredentials, () => {
+        if (!this.isUnmounting) {
+          this.setState(() => ({
+            google: window.google,
+          }));
+        }
+      });
     });
   }
 

diff --git a/...om-maps/src/__tests__/GoogleMapsLoader.js → ...s/src/__tests__/GoogleMapsLoader.jsdom.js b/...om-maps/src/__tests__/GoogleMapsLoader.js → ...s/src/__tests__/GoogleMapsLoader.jsdom.js
@@ -13,6 +13,9 @@ describe('GoogleMapsLoader', () => {
     apiKey: 'API_KEY',
   };
 
+  const flushPendingMicroTasks = () =>
+    new Promise(resolve => setImmediate(resolve));
+
   it('expect to call Google Maps API', () => {
     const children = jest.fn(x => x);
 
@@ -22,10 +25,12 @@ describe('GoogleMapsLoader', () => {
 
     shallow(<GoogleMapsLoader {...props}>{children}</GoogleMapsLoader>);
 
-    expect(injectScript).toHaveBeenLastCalledWith(
-      'https://maps.googleapis.com/maps/api/js?v=3.31&key=API_KEY',
-      expect.any(Function)
-    );
+    return flushPendingMicroTasks().then(() => {
+      expect(injectScript).toHaveBeenLastCalledWith(
+        'https://maps.googleapis.com/maps/api/js?v=3.31&key=API_KEY',
+        expect.any(Function)
+      );
+    });
   });
 
   it('expect to call Google Maps API with a custom API Key', () => {
@@ -38,10 +43,12 @@ describe('GoogleMapsLoader', () => {
 
     shallow(<GoogleMapsLoader {...props}>{children}</GoogleMapsLoader>);
 
-    expect(injectScript).toHaveBeenLastCalledWith(
-      'https://maps.googleapis.com/maps/api/js?v=3.31&key=CUSTOM_API_KEY',
-      expect.any(Function)
-    );
+    return flushPendingMicroTasks().then(() => {
+      expect(injectScript).toHaveBeenLastCalledWith(
+        'https://maps.googleapis.com/maps/api/js?v=3.31&key=CUSTOM_API_KEY',
+        expect.any(Function)
+      );
+    });
   });
 
   it('expect to call Google Maps API with a custom endpoint', () => {
@@ -55,10 +62,12 @@ describe('GoogleMapsLoader', () => {
 
     shallow(<GoogleMapsLoader {...props}>{children}</GoogleMapsLoader>);
 
-    expect(injectScript).toHaveBeenLastCalledWith(
-      'https://maps.googleapis.com/maps/api/js?v=3.32&places,geometry&key=API_KEY',
-      expect.any(Function)
-    );
+    return flushPendingMicroTasks().then(() => {
+      expect(injectScript).toHaveBeenLastCalledWith(
+        'https://maps.googleapis.com/maps/api/js?v=3.32&places,geometry&key=API_KEY',
+        expect.any(Function)
+      );
+    });
   });
 
   it("expect to render nothing when it's loading", () => {
@@ -87,7 +96,7 @@ describe('GoogleMapsLoader', () => {
       ...defaultProps,
     };
 
-    injectScript.mockImplementationOnce((endpoint, callback) => {
+    injectScript.mockImplementationOnce((_, callback) => {
       global.google = google;
       callback();
     });
@@ -96,11 +105,13 @@ describe('GoogleMapsLoader', () => {
       <GoogleMapsLoader {...props}>{children}</GoogleMapsLoader>
     );
 
-    expect(wrapper.type).not.toBe(null);
-    expect(children).toHaveBeenCalledTimes(1);
-    expect(children).toHaveBeenCalledWith(google);
+    return flushPendingMicroTasks().then(() => {
+      expect(wrapper.type).not.toBe(null);
+      expect(children).toHaveBeenCalledTimes(1);
+      expect(children).toHaveBeenCalledWith(google);
 
-    delete global.google;
+      delete global.google;
+    });
   });
 
   it('expect to not call setState when we unmount before loading is complete', () => {
@@ -119,13 +130,15 @@ describe('GoogleMapsLoader', () => {
       <GoogleMapsLoader {...props}>{children}</GoogleMapsLoader>
     );
 
-    expect(wrapper.type).not.toBe(null);
-    expect(children).not.toHaveBeenCalled();
+    return flushPendingMicroTasks().then(() => {
+      expect(wrapper.type).not.toBe(null);
+      expect(children).not.toHaveBeenCalled();
 
-    wrapper.unmount();
+      wrapper.unmount();
 
-    triggerLoadingComplete();
+      triggerLoadingComplete();
 
-    expect(children).not.toHaveBeenCalled();
+      expect(children).not.toHaveBeenCalled();
+    });
   });
 });
diff --git a/packages/react-instantsearch-dom-maps/src/__tests__/GoogleMapsLoader.node.js b/packages/react-instantsearch-dom-maps/src/__tests__/GoogleMapsLoader.node.js
@@ -0,0 +1,9 @@
+/**
+ * @jest-environment node
+ */
+
+describe('GoogleMapsLoader', () => {
+  it('expect to require the file in a Node environment', () => {
+    expect(() => require('../GoogleMapsLoader')).not.toThrowError();
+  });
+});
diff --git a/yarn.lock b/yarn.lock
@@ -1827,6 +1827,13 @@ babel-plugin-dynamic-import-node@1.1.0:
     babel-template "^6.26.0"
     babel-types "^6.26.0"
 
+babel-plugin-dynamic-import-node@2.0.0:
+  version "2.0.0"
+  resolved "https://registry.yarnpkg.com/babel-plugin-dynamic-import-node/-/babel-plugin-dynamic-import-node-2.0.0.tgz#d6fc3f6c5e3bdc34e49c15faca7ce069755c0a57"
+  dependencies:
+    babel-plugin-syntax-dynamic-import "^6.18.0"
+    object.assign "^4.1.0"
+
 babel-plugin-external-helpers@6.22.0, babel-plugin-external-helpers@^6.22.0:
   version "6.22.0"
   resolved "https://registry.yarnpkg.com/babel-plugin-external-helpers/-/babel-plugin-external-helpers-6.22.0.tgz#2285f48b02bd5dede85175caf8c62e86adccefa1"