refactor(core): CORE-08 type plugin registration and guard closed instances

test/browser/plugins.spec.js

@@ -0,0 +1,94 @@
+import { expect, test } from './fixtures.js'
+
+for (const core of ['published', 'candidate']) {
+  test(`${core}: late plugin registration preserves fulfillment without reviving a closed candidate`, async ({ page }) => {
+    await page.goto(`/test/player.html?core=${core}&chapter=published`)
+    const result = await page.evaluate(async () => {
+      const container = document.createElement('div')
+      document.body.append(container)
+      const art = new window.Artplayer({ container, url: '' })
+      const registry = art.plugins
+      let resolve
+      let disposed = 0
+      const pending = registry.add(() => new Promise(done => resolve = done))
+      art.destroy()
+      resolve({ name: 'late', destroy() {
+        disposed++
+      } })
+      const fulfilled = await pending
+      const before = registry.id
+      let calls = 0
+      let error = ''
+      try {
+        registry.add(() => {
+          calls++
+          return { name: 'afterClose' }
+        })
+      }
+      catch (caught) {
+        error = caught.message
+      }
+      return { same: fulfilled === registry, late: Object.hasOwn(registry, 'late'), disposed, calls, error, idDelta: registry.id - before }
+    })
+    expect(result).toEqual(core === 'published'
+      ? { same: true, late: true, disposed: 0, calls: 1, error: '', idDelta: 1 }
+      : { same: true, late: false, disposed: 0, calls: 0, error: 'Cannot add a plugin after ArtPlayer is destroyed', idDelta: 0 })
+  })
+}
+
+test('candidate: a plugin that closes construction prevents subsequent factories', async ({ page }) => {
+  await page.goto('/test/player.html?core=candidate&chapter=published')
+  const result = await page.evaluate(() => {
+    const container = document.createElement('div')
+    document.body.append(container)
+    let calls = 0
+    const art = new window.Artplayer({ container, url: '', plugins: [(art) => {
+      art.destroy()
+      return { name: 'closing' }
+    }, () => {
+      calls++
+      return { name: 'never' }
+    }] })
+    return { calls, id: art.plugins.id, closing: Object.hasOwn(art.plugins, 'closing'), destroyed: art.isDestroy, instances: window.Artplayer.instances.length }
+  })
+  expect(result).toEqual({ calls: 0, id: 1, closing: false, destroyed: true, instances: 0 })
+})
+
+test('candidate: constructor-owned rejection is reported once without an unhandled rejection', async ({ page }) => {
+  const warnings = []
+  page.on('console', (message) => {
+    if (message.type() === 'warning' && message.text().startsWith('Failed to initialize ArtPlayer plugin:'))
+      warnings.push(message.args()[1].evaluate(error => ({ message: error.message, same: error === window.pluginFailure })))
+  })
+  await page.goto('/test/player.html?core=candidate&chapter=published')
+  await page.evaluate(() => {
+    const container = document.createElement('div')
+    document.body.append(container)
+    window.pluginFailure = new Error('owned-plugin-rejection')
+    window.art = new window.Artplayer({ container, url: '', plugins: [() => Promise.reject(window.pluginFailure)] })
+  })
+  await expect.poll(() => warnings.length).toBe(1)
+  expect(await warnings[0]).toEqual({ message: 'owned-plugin-rejection', same: true })
+  await page.evaluate(() => window.art.destroy())
+})
+
+test.describe('mobile user-agent builtin selection (not physical-device evidence)', () => {
+  test.use({ userAgent: 'Mozilla/5.0 (Linux; Android 13) AppleWebKit/537.36 Chrome/120.0.0.0 Mobile Safari/537.36' })
+  for (const core of ['published', 'candidate']) {
+    test(`${core}: builtin order and live exclusions precede user plugins`, async ({ page }) => {
+      await page.goto(`/test/player.html?core=${core}&chapter=published`)
+      const results = await page.evaluate(() => [false, true].map((isLive) => {
+        const container = document.createElement('div')
+        document.body.append(container)
+        const art = new window.Artplayer({ container, url: '', isLive, miniProgressBar: true, lock: true, autoPlayback: true, autoOrientation: true, fastForward: true, plugins: [() => ({ name: 'user' })] })
+        const names = Object.getOwnPropertyNames(art.plugins).filter(name => !['art', 'id'].includes(name))
+        art.destroy()
+        return names
+      }))
+      expect(results).toEqual([
+        ['mini-progress-bar', 'lock', 'auto-playback', 'autoOrientation', 'fastForward', 'user'],
+        ['lock', 'autoOrientation', 'user'],
+      ])
+    })
+  }
+})

test/package/runtime.cjs

@@ -75,7 +75,7 @@ const vm = require('node:vm');
   }
   check('DIST.i18n-cjs-esm-global', languages.length > 0)
   observations.blockedDeepImports = []
-  for (const specifier of ['artplayer/dist/artplayer.js', 'artplayer/types/artplayer.d.ts', 'artplayer-plugin-chapter/dist/artplayer-plugin-chapter.js']) {
+  for (const specifier of ['artplayer/dist/artplayer.js', 'artplayer/types/artplayer.d.ts', 'artplayer-plugin-chapter/dist/artplayer-plugin-chapter.js', 'artplayer/types']) {
     let code
     try {
       require.resolve(specifier)
@@ -84,7 +84,8 @@ const vm = require('node:vm');
     assert.equal(code, 'ERR_PACKAGE_PATH_NOT_EXPORTED')
     observations.blockedDeepImports.push({ specifier, code })
   }
-  check('DIST.exports-boundary', observations.blockedDeepImports.length === 3)
+  await assert.rejects(import('artplayer/types'), error => error.code === 'ERR_PACKAGE_PATH_NOT_EXPORTED')
+  check('DIST.exports-boundary', observations.blockedDeepImports.length === 4)
   observations.resolutions = {}
   for (const specifier of ['artplayer', 'artplayer/legacy', 'artplayer-plugin-chapter', 'artplayer-plugin-chapter/legacy']) {
     const resolved = require.resolve(specifier)

test/plugins.test.js

@@ -0,0 +1,150 @@
+import assert from 'node:assert/strict'
+// eslint-disable-next-line test/no-import-node-test -- Use the repository Node runner.
+import { test } from 'node:test'
+import vm from 'node:vm'
+import { loadModules } from './helpers/load.js'
+
+const lifecycle = 'packages/artplayer/src/lifecycle/instance'
+const { Plugins, beginLifecycle, getScope, destroyInstance } = await loadModules({
+  Plugins: 'packages/artplayer/src/plugins/index',
+  ...Object.fromEntries(['beginLifecycle', 'getScope', 'destroyInstance'].map(name => [name, { file: lifecycle, name }])),
+})
+
+function create(plugins = []) {
+  const art = { option: { miniProgressBar: false, isLive: false, lock: false, autoPlayback: false, autoOrientation: false, fastForward: false, plugins } }
+  beginLifecycle(art)
+  return { art, registry: new Plugins(art) }
+}
+
+test('plugin registration keeps receiver, arguments, id, descriptors and naming precedence', () => {
+  const { art, registry } = create()
+  const result = { name: 'returnedName' }
+  function factory(value) {
+    assert.equal(this, art)
+    assert.equal(value, art)
+    assert.equal(arguments.length, 1)
+    return result
+  }
+  assert.equal(registry.add(factory), registry)
+  assert.equal(registry.returnedName, result)
+  assert.equal(registry.id, 1)
+  assert.deepEqual(Object.getOwnPropertyDescriptor(registry, 'returnedName'), { value: result, writable: false, enumerable: false, configurable: false })
+  function named() {}
+  assert.equal(registry.add(named), registry)
+  assert(Object.hasOwn(registry, 'named'))
+  registry.add((0, () => null))
+  assert.equal(registry.plugin3, null)
+  assert.throws(() => registry.add(() => result), /Cannot add a plugin that already has the same name: returnedName/)
+  assert.equal(registry.id, 4)
+  assert.deepEqual(Object.keys(registry), ['art', 'id'])
+})
+
+test('async factories preserve pending visibility, completion-time fallback and registry fulfillment', async () => {
+  const { registry } = create()
+  let resolve
+  const result = registry.add((0, () => new Promise(done => resolve = done)))
+  assert(result instanceof Promise)
+  assert(!Object.hasOwn(registry, 'plugin1'))
+  registry.add(() => ({ name: 'second' }))
+  resolve(undefined)
+  assert.equal(await result, registry)
+  assert(Object.hasOwn(registry, 'plugin2'))
+  assert(!Object.hasOwn(registry, 'plugin1'))
+  const failure = new Error('factory failure')
+  await assert.rejects(registry.add(() => Promise.reject(failure)), error => error === failure)
+  assert.throws(() => registry.add(() => {
+    throw failure
+  }), error => error === failure)
+})
+
+test('only same-realm Promises are awaited, preserving catch-less thenables and foreign promises', () => {
+  const { registry } = create()
+  const thenable = { name: 'thenable', then() {
+    throw new Error('must not assimilate')
+  } }
+  assert.equal(registry.add(() => thenable), registry)
+  assert.equal(registry.thenable, thenable)
+  const foreign = vm.runInNewContext('Promise.resolve({ name: "foreignResult" })')
+  const foreignFactory = () => foreign
+  assert.equal(registry.add(foreignFactory), registry)
+  assert.equal(registry.foreignFactory, foreign)
+})
+
+test('closed registries ignore late results without inspecting or destroying them and reject new factories', async () => {
+  const { art, registry } = create()
+  let resolve
+  const pending = registry.add(() => new Promise(done => resolve = done))
+  getScope(art).dispose()
+  resolve({ get name() {
+    throw new Error('late result must not be inspected')
+  }, destroy() {
+    throw new Error('not a lifecycle hook')
+  } })
+  assert.equal(await pending, registry)
+  assert.deepEqual(Object.getOwnPropertyNames(registry), ['art', 'id'])
+  assert.throws(() => registry.add(() => {
+    throw new Error('factory must not run')
+  }), /Cannot add a plugin after ArtPlayer is destroyed/)
+  assert.equal(registry.id, 1)
+})
+
+test('reentrant destruction in a name getter or property-key coercion cannot publish a result', () => {
+  for (const closeAt of [0, 1, 2, 3]) {
+    const { art, registry } = create()
+    const coercions = []
+    const name = { [Symbol.toPrimitive](hint) {
+      coercions.push(hint)
+      if (coercions.length === closeAt)
+        getScope(art).dispose()
+      return 'closing'
+    } }
+    const result = closeAt > 0
+      ? { name }
+      : { get name() {
+          getScope(art).dispose()
+          return 'closing'
+        } }
+    assert.equal(registry.add(() => result), registry)
+    assert(!Object.hasOwn(registry, 'closing'))
+    assert.deepEqual(coercions, Array.from({ length: closeAt }, () => 'string'))
+  }
+})
+
+test('live key coercion order and symbol-name failure retain native behavior', () => {
+  const { registry } = create()
+  const calls = []
+  const result = { name: { [Symbol.toPrimitive](hint) {
+    calls.push(hint)
+    return `key${calls.length}`
+  } } }
+  registry.add(() => result)
+  assert.equal(registry.key3, result)
+  assert.deepEqual(calls, ['string', 'string', 'string'])
+  assert.throws(() => registry.add(() => ({ name: Symbol('name') })), TypeError)
+})
+
+test('constructor stops after a synchronous close and observes its own async rejection', async (context) => {
+  const failure = new Error('initial async failure')
+  const warnings = []
+  context.mock.method(console, 'warn', (...args) => warnings.push(args))
+  const { art } = create([() => Promise.reject(failure), function close() {
+    getScope(this).dispose()
+  }, () => {
+    throw new Error('must not start')
+  }])
+  assert(getScope(art).closed)
+  await new Promise(resolve => setImmediate(resolve))
+  assert.deepEqual(warnings, [['Failed to initialize ArtPlayer plugin:', failure]])
+})
+
+test('registration during reset is rejected before resource scope disposal', () => {
+  const { art, registry } = create()
+  Object.assign(art, { isDestroy: false, reset() {
+    assert.equal(getScope(art).closed, false)
+    assert.throws(() => registry.add(() => {
+      throw new Error('must not execute during reset')
+    }), /Cannot add a plugin after ArtPlayer is destroyed/)
+  }, template: { $video: {}, destroy() {} }, emit() {} })
+  destroyInstance(art, [art], true, true)
+  assert.equal(registry.id, 0)
+})

test/types/plugins-public.ts

@@ -0,0 +1,29 @@
+import type { PluginFactory, Plugins } from 'artplayer'
+import Artplayer from 'artplayer'
+
+declare module 'artplayer/types' {
+  interface Plugins {
+    typedExtension?: { update: (count: number) => void }
+  }
+  interface Events {
+    'typed-extension:update': [count: number]
+  }
+}
+
+const factory: PluginFactory = function (art) {
+  const receiver: number = this.id
+  return { name: 'typedExtension', update(count: number) {
+    art.emit('typed-extension:update', count + receiver)
+  } }
+}
+const art = new Artplayer({ container: '#player', plugins: [factory] }, function (instance) {
+  this.plugins.typedExtension?.update(1)
+  instance.plugins.typedExtension?.update(2)
+})
+const plugins: Plugins = art.plugins
+plugins.typedExtension?.update(3)
+art.on('typed-extension:update', count => count.toFixed())
+// @ts-expect-error Installed result methods retain their argument types.
+plugins.typedExtension?.update('bad')
+// @ts-expect-error Declared event listener payloads remain checked.
+art.on('typed-extension:update', (count: string) => count)

test/types/plugins-source.ts

@@ -0,0 +1,21 @@
+import type { PluginFactory, PluginHost } from '../../packages/artplayer/src/plugins/types'
+import Plugins from '../../packages/artplayer/src/plugins'
+
+interface Host extends PluginHost<Host> {
+  id: number
+}
+declare const host: Host
+const registry = new Plugins(host)
+const sync: typeof registry = registry.add(function (art) {
+  const id: number = this.id + art.id
+  return { name: 'sync', id }
+})
+const asyncResult: Promise<typeof registry> = registry.add(async () => ({ name: 'async' }))
+const mixed: typeof registry | Promise<typeof registry> = registry.add(() => Math.random() ? Promise.resolve(1) : 1)
+const untyped: PluginFactory<Host> = () => null
+const unknown: typeof registry | Promise<typeof registry> = registry.add(untyped)
+// @ts-expect-error Synchronous plugins do not always return a Promise.
+const bad: Promise<typeof registry> = registry.add(() => ({ name: 'sync' }))
+// @ts-expect-error The factory receiver and argument must match the actual host.
+registry.add((art: { incompatible: string }) => art)
+void [sync, asyncResult, mixed, unknown, bad]