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

Dependencies: Improved getLoader #2151

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 83 additions & 64 deletions dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,22 @@ var getLoader = (function () {
var noop = function () { };

/**
* Converts the given value to an array.
* Invokes the given callback for all elements of the given value.
*
* If the given value is already an array, the value itself will be returned.
* `null` and `undefined` will return an empty array.
* For every other value a new array with the given value as its only element will be created.
* If the given value is an array, the callback will be invokes for all elements. If the given value is `null` or
* `undefined`, the callback will not be invoked. In all other cases, the callback will be invoked with the given
* value as parameter.
*
* @param {null | undefined | T | T[]} value
* @returns {T[]}
* @param {(value: T, index: number) => void} callbackFn
* @returns {void}
* @template T
*/
function toArray(value) {
function forEach(value, callbackFn) {
if (Array.isArray(value)) {
return value;
} else {
return value == null ? [] : [value];
value.forEach(callbackFn);
} else if (value != null) {
callbackFn(value, 0);
}
}

Expand All @@ -68,10 +69,10 @@ var getLoader = (function () {
* @param {Components} components
* @returns {EntryMap}
*
* @typedef {{ [id: string]: Readonly<ComponentEntry> | undefined }} EntryMap
* @typedef {{ readonly [id: string]: Readonly<ComponentEntry> | undefined }} EntryMap
*/
function createEntryMap(components) {
/** @type {EntryMap} */
/** @type {Object<string, Readonly<ComponentEntry>>} */
var map = {};

for (var categoryName in components) {
Expand All @@ -92,13 +93,14 @@ var getLoader = (function () {
* Creates a full dependencies map which includes all types of dependencies and their transitive dependencies.
*
* @param {EntryMap} entryMap
* @returns {DependencyMap}
* @returns {DependencyResolver}
*
* @typedef {Object<string, StringSet>} DependencyMap
* @typedef {(id: string) => StringSet} DependencyResolver
*/
function createDependencyMap(entryMap) {
/** @type {DependencyMap} */
function createDependencyResolver(entryMap) {
/** @type {Object<string, StringSet>} */
var map = {};
var _stackArray = [];

/**
* Adds the dependencies of the given component to the dependency map.
Expand All @@ -124,31 +126,47 @@ var getLoader = (function () {

var entry = entryMap[id];
if (entry) {
/** @type {string[]} */
var deps = [].concat(entry.require, entry.modify, entry.optional).filter(Boolean);
deps.forEach(function (depId) {
/**
* This will add the direct dependency and all of its transitive dependencies to the set of
* dependencies of `entry`.
*
* @param {string} depId
* @returns {void}
*/
function handleDirectDependency(depId) {
if (!(depId in entryMap)) {
throw new Error(id + ' depends on an unknown component ' + depId);
}
if (depId in dependencies) {
// if the given dependency is already in the set of deps, then so are its transitive deps
return;
}

addToMap(depId, stack);
dependencies[depId] = true;
for (var transitiveDepId in map[depId]) {
dependencies[transitiveDepId] = true;
}
});
}

forEach(entry.require, handleDirectDependency);
forEach(entry.optional, handleDirectDependency);
forEach(entry.modify, handleDirectDependency);
}

map[id] = dependencies;

stack.pop();
}

for (var id in entryMap) {
addToMap(id, []);
}

return map;
return function (id) {
var deps = map[id];
if (!deps) {
addToMap(id, _stackArray);
deps = map[id];
}
return deps;
};
}

/**
Expand All @@ -158,25 +176,32 @@ var getLoader = (function () {
* @returns {(idOrAlias: string) => string}
*/
function createAliasResolver(entryMap) {
/** @type {Object<string, string>} */
var map = {};

for (var id in entryMap) {
var entry = entryMap[id];
var aliases = toArray(entry && entry.alias);
aliases.forEach(function (alias) {
if (alias in map) {
throw new Error(alias + ' cannot be alias for both ' + id + ' and ' + map[alias]);
}
if (alias in entryMap) {
throw new Error(alias + ' cannot be alias of ' + id + ' because it is a component.');
}
map[alias] = id;
});
}
/** @type {Object<string, string> | undefined} */
var map;

return function (idOrAlias) {
return map[idOrAlias] || idOrAlias;
if (idOrAlias in entryMap) {
return idOrAlias;
} else {
// only create the alias map if necessary
if (!map) {
map = {};

for (var id in entryMap) {
var entry = entryMap[id];
forEach(entry && entry.alias, function (alias) {
if (alias in map) {
throw new Error(alias + ' cannot be alias for both ' + id + ' and ' + map[alias]);
}
if (alias in entryMap) {
throw new Error(alias + ' cannot be alias of ' + id + ' because it is a component.');
}
map[alias] = id;
});
}
}
return map[idOrAlias] || idOrAlias;
}
};
}

Expand All @@ -191,14 +216,14 @@ var getLoader = (function () {
* Creates an implicit DAG from the given components and dependencies and call the given `loadComponent` for each
* component in topological order.
*
* @param {DependencyMap} dependencyMap
* @param {DependencyResolver} dependencyResolver
* @param {StringSet} ids
* @param {(id: string) => T} loadComponent
* @param {LoadChainer<T>} [chainer]
* @returns {T}
* @template T
*/
function loadComponentsInOrder(dependencyMap, ids, loadComponent, chainer) {
function loadComponentsInOrder(dependencyResolver, ids, loadComponent, chainer) {
const series = chainer ? chainer.series : undefined;
const parallel = chainer ? chainer.parallel : noop;

Expand Down Expand Up @@ -228,7 +253,7 @@ var getLoader = (function () {

// all dependencies of the component in the given ids
var dependsOn = [];
for (var depId in dependencyMap[id]) {
for (var depId in dependencyResolver(id)) {
if (depId in ids) {
dependsOn.push(depId);
}
Expand Down Expand Up @@ -347,15 +372,12 @@ var getLoader = (function () {
load.forEach(addRequirements);
function addRequirements(id) {
var entry = entryMap[id];
if (entry) {
var require = toArray(entry.require);
require.forEach(function (reqId) {
if (!(reqId in loadedSet)) {
loadSet[reqId] = true;
addRequirements(reqId);
}
});
}
forEach(entry && entry.require, function (reqId) {
if (!(reqId in loadedSet)) {
loadSet[reqId] = true;
addRequirements(reqId);
}
});
}

// add components to reload
Expand All @@ -365,7 +387,7 @@ var getLoader = (function () {
// 2) x depends on a component in `load`.
// The above two condition have to be applied until nothing changes anymore.

var dependencyMap = createDependencyMap(entryMap);
var dependencyResolver = createDependencyResolver(entryMap);

/** @type {StringSet} */
var loadAdditions = loadSet;
Expand All @@ -377,20 +399,17 @@ var getLoader = (function () {
// condition 1)
for (var loadId in loadAdditions) {
var entry = entryMap[loadId];
if (entry) {
var modify = toArray(entry.modify);
modify.forEach(function (modId) {
if (modId in loadedSet) {
newIds[modId] = true;
}
});
}
forEach(entry && entry.modify, function (modId) {
if (modId in loadedSet) {
newIds[modId] = true;
}
});
}

// condition 2)
for (var loadedId in loadedSet) {
if (!(loadedId in loadSet)) {
for (var depId in dependencyMap[loadedId]) {
for (var depId in dependencyResolver(loadedId)) {
if (depId in loadSet) {
newIds[loadedId] = true;
break;
Expand All @@ -415,7 +434,7 @@ var getLoader = (function () {
return ids;
},
load: function (loadComponent, chainer) {
return loadComponentsInOrder(dependencyMap, loadSet, loadComponent, chainer);
return loadComponentsInOrder(dependencyResolver, loadSet, loadComponent, chainer);
}
};

Expand Down
13 changes: 10 additions & 3 deletions tests/dependencies-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ describe('Dependency logic', function () {
}
}
};
getLoader(circular, ['a']).getIds();
getLoader(circular, ['a', 'foo' /* force the lazy alias resolver */]).getIds();
});
});

Expand All @@ -148,7 +148,7 @@ describe('Dependency logic', function () {
b: 'B'
}
};
getLoader(circular, ['a']).getIds();
getLoader(circular, ['a', 'foo' /* force the lazy alias resolver */]).getIds();
});
});

Expand Down Expand Up @@ -248,7 +248,14 @@ describe('components.json', function () {

it('- should be valid', function () {
try {
getLoader(components, Object.keys(components.languages).filter(k => k != 'meta')).getIds();
const allIds = [];
for (const category in components) {
Object.keys(components[category]).forEach(id => allIds.push(id));
}
// and an alias, so we force the lazy alias resolver to check all aliases
allIds.push('js');

getLoader(components, allIds).getIds();
} catch (error) {
assert.fail(error.toString());
}
Expand Down