Keysystem update for wrts DRMEngine

[thomasjammet]

🚀 Features

• (Util) Add an helper to convert string or array to a normalized array
• (Connect) Support DRM template configurations :
  • add structured DRM request and certificate config types
  • extend KeySystem with templateConfigurations
  • add createTemplateConfigurations() helper for DRM capability templates
  • support robustness-only templates intended to be completed from metadata
  • add utility and tests for template generation

Other

• Add text rendering to coverage command

[codecov]

Codecov Report

❌ Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/Connect.ts	97.22%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[imarsv]

PR #73 Review — Keysystem update for wrts DRMEngine

Author: Thomas Jammet | Base: main | +268/-20 across 5 files

---

Overview

Refactors the KeySystem type to support richer DRM configuration (structured license/certificate objects, header support at the right level), and adds a createTemplateConfigurations() helper that generates cartesian-product MediaKeySystemConfiguration arrays from content types and robustness values. Also adds a normalizeStringArray() utility and improves the coverage config.

---

Breaking Changes

This PR contains multiple breaking API changes that affect downstream consumers:

• licenseUrl: string → license?: DRMRequestConfig — renamed field, type widened, and made optional. Any caller using KeySystem.licenseUrl will break.
• headers removed from KeySystem object level — now nested inside DRMRequestConfig. Callers who set top-level headers will silently have them ignored.
• audioRobustness / videoRobustness removed from KeySystem — replaced by templateConfigurations. Callers relying on these fields will break.

These are all intentional architectural improvements, but they should be flagged as a major/breaking version bump (semver major), not just reflected in the changelog.

---

Code Quality Issues

1. Misplaced test — doesn't test createTemplateConfigurations at all (Connect.spec.ts:200-211)

The first test in the createTemplateConfigurations describe block never calls the function. It only checks that the Params type still allows a string shorthand for contentProtection. Move it to a Params or KeySystem type-level test block, or remove it — the type is enforced at compile time anyway.

2. Inline comments explain WHAT, not WHY (Connect.ts:49, 72)

// If no content types are provided, we create a single configuration with robustness values only
// Create a complete configuration for each combination of audio/video content types and robustness values

Project convention: "Default to writing no comments. Only add one when the WHY is non-obvious." Both of these describe what the code does, which the code itself already shows. Remove them; the JSDoc covers the contract adequately.

3. Silent ignore of audioRobustness when no audioContentTypes is provided

In the main loop path, if audioContentTypes is empty but audioRobustness is provided, the audio robustness is silently dropped (audioRobustness local is forced to ['']). The test "should not duplicate video-only configurations when audio robustness is provided" verifies this is intentional, but the JSDoc on createTemplateConfigurations doesn't document this behavior. A caller passing audioRobustness without audioContentTypes expecting multiple configs would be confused.

4. DRMCertificateConfig object allows an empty object

export type DRMCertificateConfig =
    | string
    | Uint8Array
    | {
          url?: string;      // optional
          data?: Uint8Array; // optional
          headers?: Record<string, string | null>;
      };

Both url and data are optional simultaneously, making {} (or { headers: {...} } with no cert source) a valid value. If a certificate must come from somewhere (url or data), consider { url: string; headers?: ... } | { data: Uint8Array; headers?: ... } to enforce one of them.

5. headers type widened from Record<string, string> to Record<string, string | null>

The original was Record<string, string>. Allowing null is unusual — is this intentional (e.g. to signal header deletion)? If so, document why; if not, it should stay string.

---

What Works Well

• normalizeStringArray() is clean and well-designed. The includeEmpty parameter elegantly serves both the cartesian-product use case (needs [''] as a neutral element) and the filtering use case. Tests are thorough.
• createTemplateConfigurations() cartesian logic is correct. Traced through all test cases — results match expectations. The edge cases (robustness-only templates, video-only, base config merging) are all covered.
• vitest.config.ts improvements (text reporter + include filter) are straightforward and useful.
• JSDoc on createTemplateConfigurations is well-written and covers the intended use with requestMediaKeySystemAccess().

---

Summary

Solid implementation, good test coverage. The main asks before merging:

1. Confirm this goes out as a semver major (or that main is pre-1.0) given the breaking KeySystem changes.
2. Move or remove the misplaced first test in createTemplateConfigurations.
3. Delete the two inline WHAT comments.
4. Clarify the audioRobustness-without-audioContentTypes behavior in JSDoc.
5. Consider tightening DRMCertificateConfig to require url or data when using the object form.

[MathieuPOUX]

I think the names TemplateConfigurationsParams / createTemplateConfigurations are too generic; it’s not clear that they are DRM-related.
Also I feel the variable names lack consistency and cohesion: some are prefixed with DRM, while others are not.
Here a AI + Me proposition to fix it =>

DRMRequestConfig
DRMCertificateConfig
DRMKeySystemConfig // replace KeySystem
DRMKeySystemConfigurationParams // replace TemplateConfigurationsParams
createDRMKeySystemConfigurations // replace createTemplateConfigurations

and DRMKeySystemConfig should have its property called "configurations", it's enough clear:

export type DRMKeySystemConfig =
| string
| {
	  /**
	   * The license URL or configuration for the key system. If it's a string, it's the URL of the license server.
	   */
	  license?: DRMRequestConfig;
	  /**
	   * The certificate URL if needed (for FairPlay) or the certificate data as Uint8Array.
	   */
	  certificate?: DRMCertificateConfig;
	  /**
	   * Optional MediaKeySystemConfigurations.
	   *
	   * If metadata is available, a DRM implementation may enrich capabilities that do not define `contentType`
	   * before calling `requestMediaKeySystemAccess()`. Explicit `contentType` values provided by the user should
	   * take precedence over metadata-derived values.
	   *
	   * If metadata is not available, these configurations are expected to be complete enough to be used as-is.
	   */
	  configurations?: MediaKeySystemConfiguration[];
  };

[MathieuPOUX]

In seeing again the code, I think that it could be simplify in:

DRMRequest
DRMCertificate
DRMKeySystem
DRMKeySystemConfigurationParams
createDRMKeySystemConfigurations

[imarsv]

@claude review

[imarsv]

PR #73 — Keysystem Update for WRTS DRMEngine

Author: Thomas Jammet | Branch: keysystem-update → main | +315 / -22, 5 files

---

Overview

This PR refactors DRM type definitions in Connect.ts and adds a utility helper to simplify building MediaKeySystemConfiguration[] arrays. The main changes:

1. toStringArray utility — normalizes string | string[] | undefined to a filtered string[]
2. createMediaKeySystemConfigurations — builds all combinations of audio/video content types × robustness levels, supporting "template" configs where content types are filled in later from metadata
3. KeySystem → MediaKeySystem rename — with restructured license/certificate/configurations fields
4. vitest.config.ts — adds text reporter and include glob

---

Code Quality & Style

toStringArray (src/Util.ts) — Clean, minimal, well-tested. The defaultValue parameter is a good touch.

createMediaKeySystemConfigurations (src/Connect.ts) — Logic is correct and the tests cover all the important cases (cartesian product, unresolved audio-only, unresolved video-only, robustness-only templates). A few readability concerns:

• The dual use of audioRobustness/videoRobustness local variables is confusing: they're derived from requestedAudioRobustness/requestedVideoRobustness but with different semantics (the iteration vars can contain '' as fallback). The distinction between them and their "requested" counterparts isn't obvious at a glance.

• In the early-return robustness-only branch, the ...(robustness && { robustness }) guard is redundant — toStringArray already filters empty strings so robustness is always truthy there. It's harmless but adds noise.

Type design — MediaKeyLicense and MediaKeyCertificate union types are well-structured. The new MediaKeySystem shape (flat license/certificate/configurations) is cleaner than the old shape that mixed URL, headers, robustness directly.

---

Issues & Risks

1. vitest.config.ts — spec files included in coverage (potential bug)

include: ['src/**/*.ts'],  // includes *.spec.ts files

This instruments test files themselves for coverage, which inflates metrics meaninglessly. Should be:

include: ['src/**/!(*.spec).ts'],

or add an exclude: ['src/**/*.spec.ts']. Vitest's default exclusions may still apply, but it's worth verifying with npm run test:coverage output.

2. Breaking change — no migration notes

KeySystem is renamed to MediaKeySystem and its shape changes significantly (licenseUrl → license, removed headers/audioRobustness/videoRobustness top-level fields). Any consumer of the old KeySystem type will break silently at runtime or with TS errors. This is intentional per the PR title, but the PR description doesn't call it out explicitly. If downstream packages (web-rtc-player, etc.) use KeySystem, they'll need coordinated updates.

3. JSDoc typo in createMediaKeySystemConfigurations

 * Helper to create the MediaKeySystem.configuration from :

Should be configurations (plural) to match the actual field name. Also, the space before : is non-standard English punctuation.

4. Missing edge-case test

createMediaKeySystemConfigurations({}) (fully empty params) returns [{}] — an array containing one empty config object. This is reasonable behavior but isn't tested. Worth adding for documentation purposes.

---

Test Coverage

Coverage is thorough for both new units:

• toStringArray: 4 cases covering string, array, undefined, and fallback
• createMediaKeySystemConfigurations: 6 cases covering the cartesian product, robustness-only templates, mixed unresolved audio/video, and base config passthrough

One gap: no test for what happens when all params are omitted (createMediaKeySystemConfigurations({})).

---

Summary

Solid PR overall. The logic is correct, the tests are comprehensive, and the type restructuring is an improvement. The main things to address before merging:

1. Fix the vitest.config.ts include pattern to exclude spec files
2. Verify downstream consumers of KeySystem are updated
3. Fix the JSDoc typo (configuration → configurations)

[ceeblue-release-bot]

🎉 This PR is included in version 7.9.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀