Add plugin dependency validation and finish lifecycle

packages/animation/src/plugin.js

@@ -1,8 +1,10 @@
 import { App, Plugin } from '@wimaengine/app'
 import { AssetPlugin, Assets } from '@wimaengine/asset'
-import { AppSchedule } from '@wimaengine/core'
+import { AppSchedule, CorePlugin } from '@wimaengine/core'
 import { ComponentHooks } from '@wimaengine/ecs'
-import { typeidGeneric } from '@wimaengine/type'
+import { ReflectPlugin } from '@wimaengine/reflect'
+import { TimePlugin } from '@wimaengine/time'
+import { typeidGeneric, typeid } from '@wimaengine/type'
 import { AnimationClip } from './assets'
 import { AnimationPlayer, dropAnimationPlayer } from './components'
 import { AnimationClipAssets } from './resources'
@@ -27,4 +29,8 @@ export class AnimationPlugin extends Plugin {
       .registerSystem({ schedule: AppSchedule.Update, system: applyAnimations })
       .setResourceAlias(typeidGeneric(Assets, [AnimationClip]), AnimationClipAssets)
   }
+
+  requires() {
+    return [typeid(CorePlugin), typeid(ReflectPlugin), typeid(TimePlugin)]
+  }
 }

packages/app/src/app.js

@@ -21,34 +21,113 @@ export class PluginRegistry {
    */
   names = new Set()
 
+  /**
+   * Tracks the parent plugin for nested registrations.
+   *
+   * @type {Map<TypeId, TypeId>}
+   */
+  parents = new Map()
+
+  /**
+   * Tracks the active plugin registration stack.
+   *
+   * @type {TypeId[]}
+   */
+  stack = []
+
   /**
    * @param {Plugin} plugin
+   * @throws When a dublicate plugin is registered.
    */
   add(plugin) {
+    const pluginId = plugin.name()
+
+    if (this.names.has(pluginId)) {
+      throw new Error(`The plugin \`${pluginId}\` is already registered.${formatRegistrationTrace(this, pluginId)}`)
+    }
+
+    const parentId = this.stack[this.stack.length - 1]
+
     this.list.push(plugin)
-    this.names.add(plugin.name())
+    this.names.add(pluginId)
+
+    if (parentId === undefined) {
+      this.parents.delete(pluginId)
+    } else {
+      this.parents.set(pluginId, parentId)
+    }
   }
 
   /**
-   * @param {TypeId} plugin
+   * @param {TypeId} pluginId
    */
-  hasTypeId(plugin) {
-    this.names.has(plugin)
+  hasTypeId(pluginId) {
+    return this.names.has(pluginId)
   }
 
   /**
-   * @param {Plugin} plugin
+   * @param {TypeId} pluginId
    */
-  has(plugin) {
-    this.hasTypeId(plugin.name())
+  has(pluginId) {
+    return this.hasTypeId(pluginId)
+  }
+
+  /**
+   * @param {TypeId} pluginId
+   */
+  enter(pluginId) {
+    this.stack.push(pluginId)
+  }
+
+  leave() {
+    this.stack.pop()
   }
 
   /**
    * @param {App} app
    */
   register(app) {
     for (let i = 0; i < this.list.length; i++) {
-      this.list[i].register(app)
+      const plugin = this.list[i]
+
+      this.enter(plugin.name())
+      plugin.register(app)
+      this.leave()
+    }
+  }
+
+  /**
+   * @param {App} app
+   */
+  finish(app) {
+    for (let i = 0; i < this.list.length; i++) {
+      const plugin = this.list[i]
+
+      this.enter(plugin.name())
+      plugin.finish(app)
+      this.leave()
+    }
+  }
+
+  /**
+   * Validates that registered plugins only require plugins that already exist.
+   *
+   * @param {App} app
+   */
+  assertDependencies(app) {
+    for (let i = 0; i < this.list.length; i++) {
+      const plugin = this.list[i]
+      const pluginId = plugin.name()
+      const requires = plugin.requires()
+
+      for (let j = 0; j < requires.length; j++) {
+        const requiredId = requires[j]
+
+        assertTrue(
+          app.hasPlugin(requiredId),
+          `The plugin \`${pluginId}\` requires \`${requiredId}\`, but it is not registered.`
+        )
+      }
     }
   }
 }
@@ -118,6 +197,16 @@ export class App {
     return this.worlds.getWorld(label)
   }
 
+  /**
+   * Checks whether a plugin has been registered.
+   *
+   * @param {TypeId} pluginId
+   * @returns {boolean}
+   */
+  hasPlugin(pluginId) {
+    return this.plugins.hasTypeId(pluginId)
+  }
+
   /**
    * Creates a world by label.
    *
@@ -194,6 +283,7 @@ export class App {
    * {@link App.defaultWorld}, {@link App.setComponentHooks},
    * {@link App.setResourceAlias},
    * {@link App.setResource} and {@link App.setResourceByTypeId}.
+   * Invokes registered plugin finish callbacks after dependency validation and before queued hooks and resources are applied.
    * Applies queued resource aliases to every world before the runner starts.
    * Applies queued component hooks to every world before the runner starts.
    * Flushes any resources staged through {@link App.setResource} into their target worlds before the runner starts.
@@ -202,6 +292,8 @@ export class App {
    */
   run() {
     this.plugins.register(this)
+    this.plugins.assertDependencies(this)
+    this.plugins.finish(this)
     this.applyComponentHooks()
     this.applyResourceAliases()
     this.flushResources()
@@ -390,6 +482,20 @@ export class Plugin {
    */
   register(_app) { }
 
+  /**
+   * @param {App} _app
+   */
+  finish(_app) { }
+
+  /**
+   * Returns the type ids of plugins required by this plugin.
+   *
+   * @returns {TypeId[]}
+   */
+  requires() {
+    return []
+  }
+
   /**
    * @returns {TypeId}
    */
@@ -454,3 +560,41 @@ export class PluginGroup extends Plugin {
     }
   }
 }
+
+/**
+ * Formats the registration chain for error output.
+ *
+ * @param {PluginRegistry} registry
+ * @param {TypeId} pluginId
+ * @returns {string}
+ */
+function formatRegistrationTrace(registry, pluginId) {
+  const chain = []
+  let current = pluginId
+
+  while (current !== undefined) {
+    chain.push(current)
+    current = registry.parents.get(current)
+  }
+
+  chain.reverse()
+
+  if (!chain.length) {
+    return ''
+  }
+
+  const lines = ['Plugin registration stack:']
+
+  for (let i = 0; i < chain.length; i++) {
+    const currentId = chain[i]
+    const parentId = chain[i - 1]
+
+    if (parentId === undefined) {
+      lines.push(`- \`${currentId}\` registered directly on the app`)
+    } else {
+      lines.push(`- \`${currentId}\` registered by \`${parentId}\``)
+    }
+  }
+
+  return `\n\n${lines.join('\n')}`
+}

packages/app/tests/app.test.js

@@ -0,0 +1,152 @@
+import { test, describe } from "vitest";
+import { ok, strictEqual, throws } from "node:assert";
+import { App, Plugin } from "../src";
+import { typeid } from "@wimaengine/type";
+
+class ProvidedPlugin extends Plugin { }
+
+class ChildPlugin extends Plugin {
+  requires() {
+    return [typeid(ProvidedPlugin)]
+  }
+}
+
+class ProviderBootstrapPlugin extends Plugin {
+  register(app) {
+    app.registerPlugin(new ProvidedPlugin())
+  }
+}
+
+class ParentPlugin extends Plugin {
+  register(app) {
+    app.registerPlugin(new ChildPlugin())
+  }
+}
+
+class BootstrapPlugin extends Plugin {
+  register(app) {
+    app.registerPlugin(new ProviderBootstrapPlugin())
+  }
+}
+
+class MissingPlugin extends Plugin { }
+
+class MissingDependencyPlugin extends Plugin {
+  requires() {
+    return [typeid(MissingPlugin)]
+  }
+}
+
+class FinishResource { }
+
+class FinishPlugin extends Plugin {
+  finish(app) {
+    strictEqual(app.initialized, false)
+    app.setResource(finishResource)
+  }
+}
+
+const finishResource = new FinishResource()
+
+describe("Testing `App` plugin registry behavior", () => {
+  test("`App.hasPlugin` checks by type id", () => {
+    const app = new App()
+
+    app.registerPlugin(new ParentPlugin())
+
+    strictEqual(app.hasPlugin(typeid(ParentPlugin)), true)
+    strictEqual(app.hasPlugin(typeid(ProvidedPlugin)), false)
+  })
+
+  test("`App.run` validates plugins registered during registration", () => {
+    const app = new App()
+
+    app
+      .registerPlugin(new ParentPlugin())
+      .registerPlugin(new BootstrapPlugin())
+      .setRunner(() => {})
+      .run()
+
+    strictEqual(app.hasPlugin(typeid(ProvidedPlugin)), true)
+    strictEqual(app.hasPlugin(typeid(ChildPlugin)), true)
+  })
+
+  test("`App.registerPlugin` reports a duplicate plugin registered by another plugin", () => {
+    const app = new App()
+    const parent = new ParentPlugin()
+
+    parent.register = (app) => {
+      app.registerPlugin(new ChildPlugin())
+      app.registerPlugin(new ChildPlugin())
+    }
+
+    let error
+
+    try {
+      app
+        .registerPlugin(parent)
+        .setRunner(() => {})
+        .run()
+    } catch (caught) {
+      error = caught
+    }
+
+    ok(error instanceof Error)
+
+    const message = error.message
+    const parentId = typeid(ParentPlugin)
+    const childId = typeid(ChildPlugin)
+
+    ok(message.includes(`The plugin \`${childId}\` is already registered.`))
+    ok(message.includes("Plugin registration stack:"))
+    ok(message.includes(`- \`${parentId}\` registered directly on the app`))
+    ok(message.includes(`- \`${childId}\` registered by \`${parentId}\``))
+  })
+
+  test("`App.run` throws when a plugin dependency is missing", () => {
+    const app = new App()
+
+    app
+      .registerPlugin(new MissingDependencyPlugin())
+      .setRunner(() => {})
+
+    throws(() => app.run(), /requires .* but it is not registered/)
+  })
+
+  test("`App.registerPlugin` reports a duplicate plugin registered on app", () => {
+    const app = new App()
+
+    let error
+
+    try {
+      app
+        .registerPlugin(new ChildPlugin())
+        .registerPlugin(new ChildPlugin())
+    } catch (caught) {
+      error = caught
+    }
+
+    ok(error instanceof Error)
+
+    const message = error.message
+    const childId = typeid(ChildPlugin)
+
+    ok(message.includes(`The plugin \`${childId}\` is already registered.`))
+    ok(message.includes("Plugin registration stack:"))
+    ok(message.includes(`- \`${childId}\` registered directly on the app`))
+  })
+
+  test("`Plugin.finish` runs after dependencies are checked and before resources are flushed", () => {
+    const app = new App()
+    const world = class DefaultWorld { }
+
+    app
+      .setWorld(world)
+      .defaultWorld(world)
+      .setRunner(() => {})
+      .registerPlugin(new FinishPlugin())
+      .run()
+
+    strictEqual(app.getWorld(world).getResource(FinishResource), finishResource)
+  })
+})