Merge pull request #1441 from hypothesis/support-non-localhost-dev-url

Make it easier to test client from a different device

docs/developers/envvars.rst

@@ -10,7 +10,3 @@ build tasks.
    the sidebar app's iframe), used when the host page does not contain a
    :option:`sidebarAppUrl` setting.
    ``https://hypothes.is/app.html`` by default.
-
-.. envvar:: PACKAGE_SERVER_HOSTNAME
-
-   The hostname that is used to generate URLs to the package content server.

docs/developers/mobile.rst

@@ -15,9 +15,11 @@ tested with at least current versions of iOS Safari and Chrome for Android.
    by editing ``conf/development-app.ini`` and changing the ``host`` setting from
    ``localhost`` to ``0.0.0.0``.
 
-#. Get the IP address or hostname of your development system (``<HOSTNAME>``
+#. Get the hostname of your development system (``<HOSTNAME>``
    in the steps below). You can do this using the ``hostname`` terminal command on
-   Mac/Linux.
+   macOS/Linux.
+
+   On macOS, this will typically be something like "Bobs-MacBookPro.local".
 
    .. tip::
 
@@ -39,26 +41,11 @@ tested with at least current versions of iOS Safari and Chrome for Android.
 
       make dev
 
-#. Set the :envvar:`SIDEBAR_APP_URL` and :envvar:`PACKAGE_SERVER_HOSTNAME`
-   environment variables to load assets from this hostname and start the dev
-   server:
-
-   .. code-block:: sh
-
-      # In the client repository
-
-      # Set URL which sidebar app ("app.html") is loaded from
-      export SIDEBAR_APP_URL=http://<HOSTNAME>:5000/app.html
-      # Set hostname used when generating client asset URLs
-      export PACKAGE_SERVER_HOSTNAME=<HOSTNAME>
-
-      make dev
-
-   .. tip::
+#. Make sure the "Redirect URL" of the OAuth client associated with your
+   development client matches `<HOSTNAME>`. You can configure the OAuth clients
+   registered with h at http://localhost:5000/admin/oauthclients.
 
-      When ``make dev`` runs, it will print out the URLs used for h
-      and client assets. These should include ``<HOSTNAME>`` instead of
-      ``localhost``.
+   This step is necessary to make logging into the client work.
 
 #. On your mobile device, go to a page which has the client embedded such as
    ``http://<HOSTNAME>:3000`` or ``http://<HOSTNAME>:5000/docs/help``.

gulpfile.js

@@ -19,7 +19,6 @@ const createStyleBundle = require('./scripts/gulp/create-style-bundle');
 const manifest = require('./scripts/gulp/manifest');
 const servePackage = require('./scripts/gulp/serve-package');
 const vendorBundles = require('./scripts/gulp/vendor-bundles');
-const { useSsl } = require('./scripts/gulp/create-server');
 
 const IS_PRODUCTION_BUILD = process.env.NODE_ENV === 'production';
 const SCRIPT_DIR = 'build/scripts';
@@ -279,17 +278,6 @@ function triggerLiveReload(changedFiles) {
   debouncedLiveReload();
 }
 
-/**
- * Return the hostname that should be used when generating URLs to the package
- * content server.
- *
- * Customizing this can be useful when testing the client on different devices
- * than the one the package content server is running on.
- */
-function packageServerHostname() {
-  return process.env.PACKAGE_SERVER_HOSTNAME || 'localhost';
-}
-
 let isFirstBuild = true;
 
 /**
@@ -303,15 +291,14 @@ function generateBootScript(manifest) {
 
   const defaultSidebarAppUrl = process.env.SIDEBAR_APP_URL
     ? `${process.env.SIDEBAR_APP_URL}`
-    : 'http://localhost:5000/app.html';
+    : '{current_scheme}://{current_host}:5000/app.html';
 
   let defaultAssetRoot;
 
   if (process.env.NODE_ENV === 'production') {
     defaultAssetRoot = `https://cdn.hypothes.is/hypothesis/${version}/`;
   } else {
-    const scheme = useSsl ? 'https' : 'http';
-    defaultAssetRoot = `${scheme}://${packageServerHostname()}:3001/hypothesis/${version}/`;
+    defaultAssetRoot = `{current_scheme}://{current_host}:3001/hypothesis/${version}/`;
   }
 
   if (isFirstBuild) {
@@ -363,14 +350,15 @@ gulp.task('watch-manifest', function() {
 });
 
 gulp.task('serve-package', function() {
-  servePackage(3001, packageServerHostname());
+  servePackage(3001);
 });
 
 gulp.task('serve-live-reload', function() {
   const LiveReloadServer = require('./scripts/gulp/live-reload-server');
-  const scheme = useSsl ? 'https' : 'http';
   liveReloadServer = new LiveReloadServer(3000, {
-    clientUrl: `${scheme}://${packageServerHostname()}:3001/hypothesis`,
+    // The scheme is omitted here as the client asset server will use the same
+    // protcol (HTTP or HTTPS) as the test page server.
+    clientUrl: `//{current_host}:3001/hypothesis`,
   });
 });
 

scripts/gulp/live-reload-server.js

@@ -120,7 +120,9 @@ function LiveReloadServer(port, config) {
             });
 
             var embedScript = document.createElement('script');
-            embedScript.src = '${config.clientUrl}';
+            embedScript.src = '${
+              config.clientUrl
+            }'.replace('{current_host}', document.location.hostname);
             document.body.appendChild(embedScript);
 
             var iframeIsAdded = false;

scripts/gulp/serve-package.js

@@ -19,7 +19,7 @@ const { version } = require('../../package.json');
  * returned by the service's '/embed.js' route and included in the '/app.html'
  * app.
  */
-function servePackage(port, hostname) {
+function servePackage(port) {
   const app = express();
 
   // Enable CORS for assets so that cross-origin font loading works.
@@ -43,7 +43,7 @@ function servePackage(port, hostname) {
 
   createServer(app).listen(port, () => {
     const scheme = useSsl ? 'https' : 'http';
-    log(`Package served at ${scheme}://${hostname}:${port}/hypothesis`);
+    log(`Package served at ${scheme}://localhost:${port}/hypothesis`);
   });
 }
 

src/boot/index.js

@@ -13,9 +13,12 @@
 
 const boot = require('./boot');
 const settings = require('../shared/settings').jsonConfigsFrom(document);
+const processUrlTemplate = require('./url-template');
 
 boot(document, {
-  assetRoot: settings.assetRoot || '__ASSET_ROOT__',
+  assetRoot: processUrlTemplate(settings.assetRoot || '__ASSET_ROOT__'),
   manifest: __MANIFEST__,
-  sidebarAppUrl: settings.sidebarAppUrl || '__SIDEBAR_APP_URL__',
+  sidebarAppUrl: processUrlTemplate(
+    settings.sidebarAppUrl || '__SIDEBAR_APP_URL__'
+  ),
 });

src/boot/test/url-template-test.js

+'use strict';
+
+const processUrlTemplate = require('../url-template');
+
+describe('processUrlTemplate', () => {
+  let fakeDocument;
+
+  beforeEach(() => {
+    fakeDocument = { currentScript: null };
+  });
+
+  context('when `document.currentScript` is set', () => {
+    beforeEach(() => {
+      fakeDocument.currentScript = {
+        src: 'https://john-smith-mbp.local/hypothesis',
+      };
+    });
+
+    it('replaces {current_host} in template', () => {
+      const url = processUrlTemplate(
+        'https://{current_host}:3000/script.js',
+        fakeDocument
+      );
+      assert.equal(url, 'https://john-smith-mbp.local:3000/script.js');
+    });
+
+    it('replaces {current_scheme} in template', () => {
+      const url = processUrlTemplate(
+        '{current_scheme}://localhost/script.js',
+        fakeDocument
+      );
+      assert.equal(url, 'https://localhost/script.js');
+    });
+  });
+
+  context('when `document.currentScript` is not set', () => {
+    it('does not replace parameters', () => {
+      const url = processUrlTemplate(
+        '{current_scheme}://{current_host}:2000/style.css',
+        fakeDocument
+      );
+      assert.equal(url, '{current_scheme}://{current_host}:2000/style.css');
+    });
+  });
+});

src/boot/url-template.js

+'use strict';
+
+function currentScriptUrl(document_ = document) {
+  try {
+    const scriptEl = document_.currentScript;
+    return new URL(scriptEl.src);
+  } catch (err) {
+    return null;
+  }
+}
+
+/**
+ * Replace references to `current_host` and `current_scheme` URL template
+ * parameters with the corresponding elements of the current script URL.
+ *
+ * During local development, there are cases when the client/h needs to be accessed
+ * from a device or VM that is not the system where the development server is
+ * running. In that case, all references to `localhost` need to be replaced
+ * with the IP/hostname of the dev server.
+ */
+function processUrlTemplate(url, document_ = document) {
+  const scriptUrl = currentScriptUrl(document_);
+  if (scriptUrl) {
+    url = url.replace('{current_host}', scriptUrl.hostname);
+    url = url.replace('{current_scheme}', scriptUrl.protocol.slice(0, -1));
+  }
+  return url;
+}
+
+module.exports = processUrlTemplate;