Commit c21da38a authored by Robert Knight's avatar Robert Knight

Remove polyfill loader from the boot script

None of the existing polyfills are used in any supported browsers, and going
forwards we have tentatively agreed [1] to use regular imports for polyfills,
instead of this extra machinery.

This should make it easier to see where polyfills have come from,
and the worse ergonomics (eg. of having to add an extra import, calling a
standalone function instead of a method) haven't proven a major issue.

[1] See discussion on https://github.com/hypothesis/client/pull/4722
parent fdc6ec32
import { requiredPolyfillSets } from './polyfills';
/** @typedef {import('./polyfills').PolyfillSet} PolyfillSet */
/**
* Polyfills used by both the annotator and sidebar app.
*/
const commonPolyfills = /** @type {PolyfillSet[]} */ ([
// ES APIs
'es2017',
'es2018',
// Any other polyfills which may rely on certain ES APIs should be listed here.
]);
/** /**
* @typedef SidebarAppConfig * @typedef SidebarAppConfig
* @prop {string} assetRoot - The root URL to which URLs in `manifest` are relative * @prop {string} assetRoot - The root URL to which URLs in `manifest` are relative
...@@ -144,13 +129,6 @@ function assetURL(config, path) { ...@@ -144,13 +129,6 @@ function assetURL(config, path) {
return config.assetRoot + 'build/' + config.manifest[path]; return config.assetRoot + 'build/' + config.manifest[path];
} }
/** @param {PolyfillSet[]} needed */
function polyfillBundles(needed) {
return requiredPolyfillSets(needed).map(
set => `scripts/polyfills-${set}.bundle.js`
);
}
/** /**
* Bootstrap the Hypothesis client. * Bootstrap the Hypothesis client.
* *
...@@ -188,8 +166,7 @@ export function bootHypothesisClient(doc, config) { ...@@ -188,8 +166,7 @@ export function bootHypothesisClient(doc, config) {
config.assetRoot + 'build/boot.js' config.assetRoot + 'build/boot.js'
); );
const polyfills = polyfillBundles(commonPolyfills); const scripts = ['scripts/annotator.bundle.js'];
const scripts = [...polyfills, 'scripts/annotator.bundle.js'];
for (let path of scripts) { for (let path of scripts) {
const url = assetURL(config, path); const url = assetURL(config, path);
injectScript(doc, url, { esModule: false }); injectScript(doc, url, { esModule: false });
...@@ -219,9 +196,7 @@ export function bootSidebarApp(doc, config) { ...@@ -219,9 +196,7 @@ export function bootSidebarApp(doc, config) {
preloadURL(doc, 'fetch', config.apiUrl); preloadURL(doc, 'fetch', config.apiUrl);
preloadURL(doc, 'fetch', config.apiUrl + 'links'); preloadURL(doc, 'fetch', config.apiUrl + 'links');
const polyfills = polyfillBundles(commonPolyfills); const scripts = ['scripts/sidebar.bundle.js'];
const scripts = [...polyfills, 'scripts/sidebar.bundle.js'];
for (let path of scripts) { for (let path of scripts) {
const url = assetURL(config, path); const url = assetURL(config, path);
injectScript(doc, url, { esModule: true }); injectScript(doc, url, { esModule: true });
......
import 'core-js/es/object/entries';
import 'core-js/es/object/values';
import 'core-js/es/promise/finally';
/**
* Checkers to test which polyfills are required by the current browser.
*
* This module executes in an environment without any polyfills loaded so it
* needs to run in old browsers, down to IE 11.
*
* See gulpfile.js for details of how to add a new polyfill.
*/
/**
* Return true if `obj` has all of the methods in `methods`.
*
* @param {any} obj
* @param {string[]} methods
*/
function hasMethods(obj, ...methods) {
return methods.every(method => typeof obj[method] === 'function');
}
/**
* Map of polyfill set name to function to test whether the current browser
* needs that polyfill set.
*
* Each checker function returns `true` if the polyfill is required or `false`
* if the browser has the functionality natively available.
*/
const needsPolyfill = {
es2017: () => {
return !hasMethods(Object, 'entries', 'values');
},
es2018: () => {
return (
typeof Promise !== 'function' || !hasMethods(Promise.prototype, 'finally')
);
},
};
/**
* Name of a polyfill set that can be loaded.
*
* @typedef {keyof needsPolyfill} PolyfillSet
*/
/**
* Return the subset of polyfill sets from `needed` which are needed by the
* current browser.
*
* @param {PolyfillSet[]} needed
*/
export function requiredPolyfillSets(needed) {
return needed.filter(set => {
const checker = needsPolyfill[set];
if (!checker) {
throw new Error(`Unknown polyfill set "${set}"`);
}
return checker();
});
}
import { requiredPolyfillSets } from '../';
function stubOut(obj, property, replacement = undefined) {
const saved = obj[property];
// We don't use `delete obj[property]` here because that isn't allowed for
// some native APIs in some browsers.
obj[property] = replacement;
return () => {
obj[property] = saved;
};
}
describe('boot/polyfills/index', () => {
describe('requiredPolyfillSets', () => {
let undoStub;
afterEach(() => {
if (undoStub) {
undoStub();
undoStub = null;
}
});
[
{
set: 'es2017',
providesMethod: [Object, 'entries'],
},
{
set: 'es2018',
providesMethod: [Promise.prototype, 'finally'],
},
{
set: 'es2018',
providesMethod: [window, 'Promise'],
},
].forEach(({ set, providesMethod }) => {
it(`includes "${set}" if required`, () => {
const [obj, method, replacement] = providesMethod;
undoStub = stubOut(obj, method, replacement);
const sets = requiredPolyfillSets([set]);
assert.deepEqual(sets, [set]);
});
it(`does not include "${set}" if not required`, () => {
assert.deepEqual(requiredPolyfillSets([set]), []);
});
});
});
});
import { bootHypothesisClient, bootSidebarApp, $imports } from '../boot'; import { bootHypothesisClient, bootSidebarApp } from '../boot';
function assetURL(url) { function assetURL(url) {
return `https://marginal.ly/client/build/${url}`; return `https://marginal.ly/client/build/${url}`;
} }
describe('bootstrap', () => { describe('bootstrap', () => {
let fakePolyfills;
let iframe; let iframe;
beforeEach(() => { beforeEach(() => {
iframe = document.createElement('iframe'); iframe = document.createElement('iframe');
document.body.appendChild(iframe); document.body.appendChild(iframe);
fakePolyfills = {
requiredPolyfillSets: sinon.stub().returns([]),
};
$imports.$mock({
'./polyfills': fakePolyfills,
});
}); });
afterEach(() => { afterEach(() => {
$imports.$restore();
iframe.remove(); iframe.remove();
}); });
function runBoot(app = 'annotator') { function runBoot(app = 'annotator') {
const assetNames = [ const assetNames = [
// Polyfills
'scripts/polyfills-es2017.bundle.js',
// Annotation layer // Annotation layer
'scripts/annotator.bundle.js', 'scripts/annotator.bundle.js',
'styles/annotator.css', 'styles/annotator.css',
...@@ -168,25 +155,6 @@ describe('bootstrap', () => { ...@@ -168,25 +155,6 @@ describe('bootstrap', () => {
assert.deepEqual(findAssets(iframe.contentDocument), []); assert.deepEqual(findAssets(iframe.contentDocument), []);
}); });
it('loads polyfills if required', () => {
fakePolyfills.requiredPolyfillSets.callsFake(sets =>
sets.filter(s => s.match(/es2017/))
);
runBoot('annotator');
const polyfillsLoaded = findAssets(iframe.contentDocument).filter(a =>
a.src.match(/polyfills/)
);
assert.called(fakePolyfills.requiredPolyfillSets);
assert.deepEqual(polyfillsLoaded, [
{
src: assetURL('scripts/polyfills-es2017.bundle.1234.js'),
type: 'script',
},
]);
});
}); });
describe('bootSidebarApp', () => { describe('bootSidebarApp', () => {
...@@ -219,24 +187,5 @@ describe('bootstrap', () => { ...@@ -219,24 +187,5 @@ describe('bootstrap', () => {
assert.equal(preloadLinks[1].as, 'fetch'); assert.equal(preloadLinks[1].as, 'fetch');
assert.equal(preloadLinks[1].crossOrigin, 'anonymous'); assert.equal(preloadLinks[1].crossOrigin, 'anonymous');
}); });
it('loads polyfills if required', () => {
fakePolyfills.requiredPolyfillSets.callsFake(sets =>
sets.filter(s => s.match(/es2017/))
);
runBoot('sidebar');
const polyfillsLoaded = findAssets(iframe.contentDocument).filter(a =>
a.src.match(/polyfills/)
);
assert.called(fakePolyfills.requiredPolyfillSets);
assert.deepEqual(polyfillsLoaded, [
{
src: assetURL('scripts/polyfills-es2017.bundle.1234.js'),
type: 'module',
},
]);
});
}); });
}); });
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