Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading dependencies and switching build to all rollup #2149

Merged
merged 19 commits into from
Apr 1, 2020
9 changes: 0 additions & 9 deletions minify-extras.sh

This file was deleted.

28 changes: 12 additions & 16 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,23 @@
"dist"
],
"devDependencies": {
"bluebird": "^3.5.1",
"construct-style-sheets-polyfill": "^2.1.0",
"@rollup/plugin-replace": "^2.3.1",
"bluebird": "^3.7.2",
"construct-style-sheets-polyfill": "^2.3.5",
"esm": "^3.2.25",
"mocha": "^5.2.0",
"mocha": "^7.1.1",
"opn": "^6.0.0",
"rollup": "^0.64.1",
"rollup-plugin-replace": "^2.0.0",
"terser": "^3.8.1",
"whatwg-fetch": "^2.0.4"
"rimraf": "^3.0.2",
"rollup": "^2.2.0",
"rollup-plugin-terser": "^5.3.0",
"terser": "^4.6.7",
"whatwg-fetch": "^3.0.0"
},
"scripts": {
"build": "npm run build:systemjs && npm run build:sjs && npm run min && npm run build:extras",
"build:systemjs": "rollup -c",
"build:sjs": "rollup -c --environment sjs",
"build:extras": "bash minify-extras.sh",
"min": "npm run min:systemjs && npm run min:sjs",
"min:systemjs": "cd dist && terser system.js -c passes=2 -m --source-map --comments \"/SystemJS \\d/\" -o system.min.js",
"min:sjs": "cd dist && terser s.js -c passes=2 -m --source-map -o s.min.js",
"build": "rimraf dist && rollup -c",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should retain a build mode / option that allows skipping minification here for quick builds in development.

I seem to recall there may even be a way with RollupJS to pass options, but can't remember if that is correct. Otherwise environment variables can work too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can with rollup's --environment flag

"footprint": "npm run footprint:systemjs && npm run footprint:sjs",
"footprint:systemjs": "terser dist/system.js -c passes=2 -m | gzip -9f | wc -c",
"footprint:sjs": "terser dist/s.js -c passes=2 -m | gzip -9f | wc -c",
"footprint:systemjs": "cat dist/system.min.js | gzip -9f | wc -c",
"footprint:sjs": "cat dist/s.min.js | gzip -9f | wc -c",
"test": "mocha -b -r esm test/import-map.js test/system-core.js test/url-resolution.js && npm run test-browser",
"test-browser": "node test/server.cjs",
"test-browser-watch": "WATCH_MODE=true node test/server.cjs",
Expand Down
122 changes: 106 additions & 16 deletions rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,113 @@
import replace from 'rollup-plugin-replace';
import replace from '@rollup/plugin-replace';
import fs from 'fs';
import path from 'path';
import { terser } from 'rollup-plugin-terser';

const version = JSON.parse(fs.readFileSync('package.json')).version;
const name = process.env.sjs ? 's' : 'system';
const extras = fs.readdirSync(path.resolve(__dirname, 'src/extras'));

export default {
input: `src/${name}.js`,
const terserOptions = {
mangle: {
eval: true,
module: true,
safari10: true,
toplevel: true
},
parse: {
},
compress: {
unsafe: true,
arguments: true,
hoist_funs: true,
hoist_props: true,
keep_fargs: false,
negate_iife: true,
module: true,
pure_getters: true,
passes: 2,
sequences: 400,
toplevel: true,
unsafe_proto: true,
unsafe_regexp: true,
unsafe_math: true,
unsafe_symbols: true,
unsafe_comps: true,
unsafe_Function: true,
unsafe_undefined: true
},
output: {
format: 'iife',
strict: false,
file: `dist/${name}.js`,
banner: process.env.sjs ? `/*
* SJS ${version}
* Minimal SystemJS Build
*/` : `/*
* SystemJS ${version}
*/`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation here needs to shift back two spaces.

comments(node, comment) {
return /^\* SystemJS [0-9]\./.test(comment.value.trim());
},
safari10: true
},
plugins: [replace({
TRACING: process.env.sjs ? 'false' : 'true'
})]
ecma: 5, // specify one of: 5, 2015, 2016, 2017 or 2018
keep_classnames: false,
keep_fnames: false,
ie8: false,
module: true,
nameCache: null, // or specify a name cache object
safari10: true,
toplevel: true,
warnings: false
};

export default [
mainConfig('system', true),
mainConfig('system', false),
mainConfig('s', true),
mainConfig('s', false),
...extrasConfig(true),
...extrasConfig(false)
];

function mainConfig(name, isDev) {
const sjs = name === 's';
let banner;
if (sjs) {
banner = isDev ? `/*
* SJS ${version}
* Minimal SystemJS Build
*/` : null;
} else {
banner = `/*
* SystemJS ${version}
*/`;
}

return {
input: `src/${name}.js`,
output: {
file: `dist/${name}${isDev ? '' : '.min'}.js`,
format: 'iife',
strict: false,
sourcemap: !isDev,
banner
},
plugins: [
replace({
TRACING: sjs ? 'false' : 'true',
}),
!isDev && terser(terserOptions)
]
};
}

function extrasConfig(isDev) {
return extras.map(extra => {
extra = extra.replace('.js', '');
return {
input: `src/extras/${extra}.js`,
output: {
file: `dist/extras/${extra}${isDev ? '' : '.min'}.js`,
format: 'iife',
strict: false,
compact: true,
sourcemap: !isDev
},
plugins: [
!isDev && terser(terserOptions)
]
};
});
}
7 changes: 4 additions & 3 deletions src/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,13 @@ function targetWarning (match, target, msg) {
}

export function resolveImportMap (importMap, resolvedOrPlain, parentUrl) {
let scopeUrl = parentUrl && getMatch(parentUrl, importMap.scopes);
const scopes = importMap.scopes;
let scopeUrl = parentUrl && getMatch(parentUrl, scopes);
while (scopeUrl) {
const packageResolution = applyPackages(resolvedOrPlain, importMap.scopes[scopeUrl]);
const packageResolution = applyPackages(resolvedOrPlain, scopes[scopeUrl]);
if (packageResolution)
return packageResolution;
scopeUrl = getMatch(scopeUrl.slice(0, scopeUrl.lastIndexOf('/')), importMap.scopes);
scopeUrl = getMatch(scopeUrl.slice(0, scopeUrl.lastIndexOf('/')), scopes);
}
return applyPackages(resolvedOrPlain, importMap.imports) || resolvedOrPlain.indexOf(':') !== -1 && resolvedOrPlain;
}
21 changes: 9 additions & 12 deletions src/extras/global.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
* (Included by default in system.js build)
*/
(function (global) {
const systemJSPrototype = global.System.constructor.prototype;
const isIE = navigator.userAgent.indexOf('Trident') !== -1;
const systemJSPrototype = System.constructor.prototype;

// safari unpredictably lists some new globals first or second in object order
let firstGlobalProp, secondGlobalProp, lastGlobalProp;
Expand All @@ -14,11 +13,7 @@
let lastProp;
for (let p in global) {
// do not check frames cause it could be removed during import
if (
!global.hasOwnProperty(p)
|| (!isNaN(p) && p < global.length)
|| (isIE && global[p] && global[p].parent === window)
)
if (shouldSkipProperty(p))
continue;
if (cnt === 0 && p !== firstGlobalProp || cnt === 1 && p !== secondGlobalProp)
return p;
Expand All @@ -35,11 +30,7 @@
firstGlobalProp = secondGlobalProp = undefined;
for (let p in global) {
// do not check frames cause it could be removed during import
if (
!global.hasOwnProperty(p)
|| (!isNaN(p) && p < global.length)
|| (isIE && global[p] && global[p].parent === window)
)
if (shouldSkipProperty(p))
continue;
if (!firstGlobalProp)
firstGlobalProp = p;
Expand Down Expand Up @@ -88,4 +79,10 @@
};
}];
};

function shouldSkipProperty(p) {
return !global.hasOwnProperty(p)
|| (!isNaN(p) && p < global.length)
|| (navigator.userAgent.indexOf('Trident') !== -1 && global[p] && global[p].parent === window);
}
})(typeof self !== 'undefined' ? self : global);
10 changes: 7 additions & 3 deletions src/features/import-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ import { systemJSPrototype } from '../system-core.js';
let importMap = { imports: {}, scopes: {} }, importMapPromise;

if (hasDocument) {
Array.prototype.forEach.call(document.querySelectorAll('script[type="systemjs-importmap"][src]'), function (script) {
iterateImportMaps(function (script) {
script._j = fetch(script.src).then(function (res) {
return res.json();
});
});
}, '[src]');
}

systemJSPrototype.prepareImport = function () {
if (!importMapPromise) {
importMapPromise = Promise.resolve();
if (hasDocument)
Array.prototype.forEach.call(document.querySelectorAll('script[type="systemjs-importmap"]'), function (script) {
iterateImportMaps(function (script) {
importMapPromise = importMapPromise.then(function () {
return (script._j || script.src && fetch(script.src).then(function (resp) { return resp.json(); }) || Promise.resolve(JSON.parse(script.innerHTML)))
.then(function (json) {
Expand All @@ -47,3 +47,7 @@ systemJSPrototype.resolve = function (id, parentUrl) {
function throwUnresolved (id, parentUrl) {
throw Error("Unable to resolve specifier '" + id + (parentUrl ? "' from " + parentUrl : "'"));
}

function iterateImportMaps(cb, extraSelector) {
[].forEach.call(document.querySelectorAll('script[type="systemjs-importmap"]' + (extraSelector || '')), cb);
}
14 changes: 7 additions & 7 deletions src/features/script-load.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ systemJSPrototype.createScript = function (url) {
};

let lastWindowErrorUrl, lastWindowError;
if (hasDocument)
window.addEventListener('error', function (evt) {
lastWindowErrorUrl = evt.filename;
lastWindowError = evt.error;
});

systemJSPrototype.instantiate = function (url, firstParentUrl) {
const loader = this;
return new Promise(function (resolve, reject) {
Expand All @@ -49,12 +43,18 @@ systemJSPrototype.instantiate = function (url, firstParentUrl) {
};

if (hasDocument) {
window.addEventListener('error', function (evt) {
lastWindowErrorUrl = evt.filename;
lastWindowError = evt.error;
});

window.addEventListener('DOMContentLoaded', loadScriptModules);
loadScriptModules();
}


function loadScriptModules() {
Array.prototype.forEach.call(
[].forEach.call(
document.querySelectorAll('script[type=systemjs-module]'), function (script) {
if (script.src) {
System.import(script.src.slice(0, 7) === 'import:' ? script.src.slice(7) : resolveUrl(script.src, baseUrl));
Expand Down
11 changes: 3 additions & 8 deletions src/features/worker-load.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,9 @@ import { hasSelf } from '../common';
if (hasSelf && typeof importScripts === 'function')
systemJSPrototype.instantiate = function (url) {
const loader = this;
return new Promise(function (resolve, reject) {
try {
importScripts(url);
}
catch (e) {
reject(e);
}
resolve(loader.getRegister());
return Promise.resolve().then(function () {
importScripts(url);
return loader.getRegister();
});
};

5 changes: 3 additions & 2 deletions src/system-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export { systemJSPrototype, REGISTRY }
const hasSymbol = typeof Symbol !== 'undefined';
const toStringTag = hasSymbol && Symbol.toStringTag;
const REGISTRY = hasSymbol ? Symbol() : '@';
const __esModule = '__esModule';

function SystemJS () {
this[REGISTRY] = {};
Expand Down Expand Up @@ -110,8 +111,8 @@ function getOrCreateLoad (loader, id, firstParentUrl) {
}
}

if (name.__esModule) {
ns.__esModule = name.__esModule;
if (name[__esModule]) {
ns[__esModule] = name[__esModule];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessary because it shouldn't affect the gzip size.

}
}
if (changed)
Expand Down