Commit 047a34d0 authored by Robert Knight's avatar Robert Knight

Use custom name for global `require` function

Browserify bundles that make their modules available for other bundles
to consume define a global `require` function. This is then imported by
other bundles and used to resolve module lookups.

This `require` name can clash with web pages that use eg. `RequireJS`.
Most of the time this isn't a problem because all of the page's own code
has run by the time Hypothesis loads. If however the page loads
additional scripts after Hypothesis has been loaded, they will end up
finding Hypothesis's `require` instead of their own.

This commit resolves the issue by renaming the global `require` function
to `hypothesisRequire` in two steps:

 - The `externalRequireName` Browserify option changes the _exported_
   name.
 - A custom transform stream post-processes the Browserify bundle code
   to change the _imported_ name, since Browserify doesn't have an
   option for this itself.

A small fix was needed to the `boot.js` script build to avoid stripping
lines _after_ the sourcemap URL comment, instead of just that comment.

Fixes #779
parent ddad3e3b
...@@ -287,7 +287,7 @@ function generateBootScript(manifest) { ...@@ -287,7 +287,7 @@ function generateBootScript(manifest) {
.pipe(replace('__SIDEBAR_APP_URL__', defaultSidebarAppUrl)) .pipe(replace('__SIDEBAR_APP_URL__', defaultSidebarAppUrl))
// Strip sourcemap link. It will have been invalidated by the previous // Strip sourcemap link. It will have been invalidated by the previous
// replacements and the bundle is so small that it isn't really valuable. // replacements and the bundle is so small that it isn't really valuable.
.pipe(replace(/^\/\/# sourceMappingURL=[^ ]+$/m,'')) .pipe(replace(/^\/\/# sourceMappingURL=\S+$/m,''))
.pipe(rename('boot.js')) .pipe(rename('boot.js'))
.pipe(gulp.dest('build/')); .pipe(gulp.dest('build/'));
} }
......
...@@ -12,6 +12,7 @@ const coffeeify = require('coffeeify'); ...@@ -12,6 +12,7 @@ const coffeeify = require('coffeeify');
const exorcist = require('exorcist'); const exorcist = require('exorcist');
const gulpUtil = require('gulp-util'); const gulpUtil = require('gulp-util');
const mkdirp = require('mkdirp'); const mkdirp = require('mkdirp');
const through = require('through2');
const uglifyify = require('uglifyify'); const uglifyify = require('uglifyify');
const watchify = require('watchify'); const watchify = require('watchify');
...@@ -28,6 +29,51 @@ function waitForever() { ...@@ -28,6 +29,51 @@ function waitForever() {
return new Promise(function () {}); return new Promise(function () {});
} }
/**
* Create a transform stream which wraps code from the input with a function
* which is immediately executed (aka. an "IIFE").
*
* @param {string} headerCode - Code added at the start of the wrapper function.
* @param {string} trailerCode - Code added at the end of the wrapper function.
* @return {Transform} - A Node `Transform` stream.
*/
function wrapCodeWithFunction(headerCode, trailerCode='') {
const iifeStart = '(function() {' + headerCode + ';';
const iifeEnd = ';' + trailerCode + '})()';
let isFirstChunk = true;
return through(function (data, enc, callback) {
if (isFirstChunk) {
isFirstChunk = false;
this.push(Buffer.from(iifeStart));
}
this.push(data);
callback();
}, function (callback) {
this.push(Buffer.from(iifeEnd));
callback();
});
}
/**
* Wrap a Browserify bundle's code to change the name of the `require` function
* which the bundle uses to load modules defined in other bundles.
*
* Use this together with Browserify's `externalRequireName` option to define/use
* a different name for the `require` function. This is useful to avoid conflicts
* with other code on the page which define/use "require".
*
* @param {string} name - Replacement name for the `require` function.
* @return {Transform} - A node `Transform` stream.
*/
function useExternalRequireName(name) {
// Make the `require` lookup inside the bundle find `name` in the global
// scope exported by a previous bundle, instead of `require`.
return wrapCodeWithFunction(
`var require=("function"==typeof ${name}&&${name})`
);
}
/** /**
* type Transform = 'coffee'; * type Transform = 'coffee';
* *
...@@ -65,6 +111,11 @@ module.exports = function createBundle(config, buildOpts) { ...@@ -65,6 +111,11 @@ module.exports = function createBundle(config, buildOpts) {
buildOpts = buildOpts || {watch: false}; buildOpts = buildOpts || {watch: false};
// Use a custom name for the "require" function that bundles use to export
// and import modules from other bundles. This avoids conflicts with eg.
// pages that use RequireJS.
const externalRequireName = 'hypothesisRequire';
const bundleOpts = { const bundleOpts = {
debug: true, debug: true,
extensions: ['.coffee'], extensions: ['.coffee'],
...@@ -86,6 +137,7 @@ module.exports = function createBundle(config, buildOpts) { ...@@ -86,6 +137,7 @@ module.exports = function createBundle(config, buildOpts) {
'_process', '_process',
'querystring', 'querystring',
], ],
externalRequireName,
insertGlobalVars: { insertGlobalVars: {
// The Browserify polyfill for the `Buffer` global is large and // The Browserify polyfill for the `Buffer` global is large and
// unnecessary, but can get pulled into the bundle by modules that can // unnecessary, but can get pulled into the bundle by modules that can
...@@ -179,8 +231,10 @@ module.exports = function createBundle(config, buildOpts) { ...@@ -179,8 +231,10 @@ module.exports = function createBundle(config, buildOpts) {
b.on('error', function (err) { b.on('error', function (err) {
log('Build error', err.toString()); log('Build error', err.toString());
}); });
const stream = b.pipe(exorcist(sourcemapPath)) const stream = (b
.pipe(output); .pipe(useExternalRequireName(externalRequireName))
.pipe(exorcist(sourcemapPath))
.pipe(output));
return streamFinished(stream); return streamFinished(stream);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment