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

PRO-6660: modules order #4745

Merged
merged 7 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

## UNRELEASED

### Fixes
### Adds

* Modules can now have a `before: "module-name"` property in their configuration to run (initialization) before another module.

### Fixes

* Modifies the `AposAreaMenu.vue` component to set the `disabled` attribute to `true` if the max number of widgets have been added in an area with `expanded: true`.

## 4.8.0 (2024-10-03)
Expand Down
83 changes: 82 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,90 @@ async function apostrophe(options, telemetry, rootSpan) {
return synth;
}

// Reorder modules based on their `before` property.
function sortModules(moduleNames) {
const definitions = self.options.modules;
// The module names that have a `before` property
const beforeModules = [];
// The metadata quick access of all modules
const modules = {};
// Recursion guard
const recursionGuard = {};
// The sorted modules result
const sorted = [];

// The base module sort metadata
for (const name of moduleNames) {
if (definitions[name].before) {
beforeModules.push(name);
}
modules[name] = {
before: definitions[name].before,
beforeSelf: []
};
}

// Loop through the modules that have a `before` property,
// validate and fill the initial `beforeSelf` metadata (first pass).
for (const name of beforeModules) {
const m = modules[name];
const before = m.before;
if (m.before === name) {
throw new Error(`Module "${name}" has a 'before' property that references itself. Skipping.`);
myovchev marked this conversation as resolved.
Show resolved Hide resolved
}
if (!modules[before]) {
throw new Error(`Module "${name}" has a 'before' property that references a non-existent module: "${before}". Skipping.`);
}
// Add the current module name to the target's beforeSelf.
modules[before].beforeSelf.push(name);
}

// Loop through the modules that have a `before` properties
// now that we have the initial metadata (second pass).
// This takes care of edge cases like `before` that points to another module
// that has a `before` property itself, circular `before` references, etc.
// in a very predictable way.
for (const name of beforeModules) {
const m = modules[name];
const target = modules[m.before];
if (!target) {
continue;
}
// Add all the modules that want to be before this one to the target's beforeSelf.
// Do this recursively for every module from the beforeSelf array that has own `beforeSelf` members.
addBeforeSelfRecursive(name, m.beforeSelf, target.beforeSelf);
}

// Fill in the sorted array, first wins when uniquefy-ing.
for (const name of moduleNames) {
sorted.push(...modules[name].beforeSelf, name);
}

// A unique array of sorted module names.
return [ ...new Set(sorted) ];

function addBeforeSelfRecursive(moduleName, beforeSelf, target) {
if (beforeSelf.length === 0) {
return;
}
if (recursionGuard[moduleName]) {
return;
}
recursionGuard[moduleName] = true;

beforeSelf.forEach((name) => {
if (recursionGuard[name]) {
return;
}
target.unshift(name);
addBeforeSelfRecursive(name, modules[name].beforeSelf, target);
});
}
}

async function instantiateModules() {
self.modules = {};
for (const item of modulesToBeInstantiated()) {
for (const item of sortModules(modulesToBeInstantiated())) {
// module registers itself in self.modules
const module = await self.synth.create(item, { apos: self });
await module.emit('moduleReady');
Expand Down
1 change: 1 addition & 0 deletions lib/moog.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ module.exports = function(options) {
'beforeSuperClass',
'init',
'afterAllSections',
'before',
'extend',
'improve',
'methods',
Expand Down
132 changes: 132 additions & 0 deletions test/modules-order.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
const assert = require('node:assert/strict');
const t = require('../test-lib/test.js');

describe('Modules Order', function() {

let apos;

this.timeout(t.timeout);

after(async function () {
await t.destroy(apos);
apos = null;
});

it('should sort modules based on their "before" property', async function() {
apos = await t.create({
root: module,
modules: {
// Before third, but also before first and schema (recursion)
strange: {
before: 'third',
init() { }
},
// before schema
first: {
before: '@apostrophecms/schema',
init() { }
},
// before schema, but after first (keep order)
second: {
before: '@apostrophecms/schema',
init() { }
},
// before first, but also before schema (recursion)
third: {
before: 'first',
init() { }
},
// before @apostrophecms/image (`before` in a module definition)
'test-before': {},
usual: {
init() { }
}
}
});

const expected = [
'strange',
'third',
'first',
'second',
'@apostrophecms/schema',
'test-before',
'@apostrophecms/image',
'usual'
];
const actual = Object.keys(apos.modules).filter(m => expected.includes(m));
assert.deepEqual(actual, expected, 'modules are not sorted as expected');

// We don't mess with the module owned context.
assert.strictEqual(apos.modules.first.before, undefined);

// Ensure we have the same number of modules when not reordering
const expectedModuleCount = apos.modules.length;
await t.destroy(apos);

apos = await t.create({
root: module,
modules: {
strange: {
init() { }
},
first: {
init() { }
},
second: {
init() { }
},
third: {
init() { }
},
'test-before': {
before: null
},
usual: {
init() { }
}
}
});
assert.strictEqual(apos.modules.length, expectedModuleCount, 'module count is not the same');

{
// ...and the natural order should be preserved
const expected = [
'@apostrophecms/schema',
'@apostrophecms/image',
'strange',
'first',
'second',
'third',
'test-before',
'usual'
];
const actual = Object.keys(apos.modules).filter(m => expected.includes(m));
assert.deepEqual(actual, expected, 'modules are not matching the natural order');
}
});

it('should handle circular "before" references', async function () {
await t.destroy(apos);
apos = await t.create({
root: module,
modules: {
first: {
before: 'second',
init() { }
},
second: {
before: 'first',
init() { }
}
}
});

const expected = [
'second',
'first'
];
const actual = Object.keys(apos.modules).filter(m => expected.includes(m));
assert.deepEqual(actual, expected, 'modules are not sorted as expected');
});
});
4 changes: 4 additions & 0 deletions test/modules/test-before/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module.exports = {
before: '@apostrophecms/image',
init() {}
};
Loading