`ExternalStatePlugin` allows overrides by KVSRoyal · Pull Request #817 · player-ui/player

KVSRoyal · 2026-03-13T17:58:46Z

This is now the jumbo PR covering all 3 platforms. (Was previously just for core.)

What

Please review the release notes that are part of the PR. (Putting them there helps me keep redundancy down and keep the docs accurate.)

Why

Deterministic Overrides: Users want predictable behaviour when multiple plugins or handlers are registered. The registry ensures that:

More specific matchers always take precedence (e.g., { ref: "x", type: "y" } beats { ref: "x" })
Later registrations replace earlier ones with the same specificity
Behaviour is consistent regardless of registration order

Existing Patterns: We chose to reuse Registry as this follows established patterns in Player for Asset registration. Hopefully this will make it easier for users to understand the logic.

Visibility: We chose to add debug logging when handlers are overridden to make it clear to developers what's happening during plugin composition.

Change Type (required)

Indicate the type of change your pull request is:

patch
minor
major
N/A (part of 1.0)

Does your PR have any documentation updates?

Updated docs
No Update needed
Unable to update docs

Release Notes

`ExternalActionPlugin` renamed to `ExternalStatePlugin` with new handler API

The plugin has been renamed from ExternalActionPlugin to ExternalStatePlugin. The name ExternalState better reflects what the plugin does — it handles EXTERNAL navigation states — and is now used consistently across all platforms.

The handler API has changed. Instead of a single function that receives all external states, you now provide an array of handlers. Each handler requires a ref (matching the external state's ref field) and a handlerFunction. An optional match object can be provided to match on additional state properties beyond ref. See docs site for full examples.

Handler matching and specificity

Handlers are matched against the external state using partial object matching on ref and any additional properties in match. When multiple handlers could match a state, the most specific one wins (the one with the most matching properties).

This allows you to register a general handler for a ref and a more specific one for a particular variant:

new ExternalStatePlugin([
  // Matches any state with ref: "my-action"
  { ref: "my-action", handlerFunction: () => "default" },
  // Takes precedence when type: "special" is also present
  {
    ref: "my-action",
    match: { type: "special" },
    handlerFunction: () => "special",
  },
]);

Overriding handlers across plugins

Overriding handlers is now supported. When multiple ExternalStatePlugin instances register a handler for the same state, the last registered handler wins. A debug log is emitted when a handler is replaced, so you can tell when an override is happening.

To add more handlers after initial setup, register a new ExternalStatePlugin instance. Plugins are typically grouped together and easy to find by name. This makes it straightforward to determine the override order.

We considered allowing more handlers to be added to an ExternalStatePlugin after creation, however, this would make it more difficult to figure out the override order. So we do not support that.

Missing handlers

For now, If no handler matches an external state, the Player remains on that state and no transition occurs. Ensure every EXTERNAL state in your flow has a registered handler.

Note

In a future release, unhandled external states will automatically report an error via the Error Controller.

KetanReddy · 2026-03-13T21:15:45Z

Fix bug with sorted-array library type def (incorrect compare function and type definitions)

Maybe out of scope for this PR but sorted-array is deprecated so we might want to replace it?

KVSRoyal · 2026-03-17T18:27:42Z

Reopening to see if I can trigger Claude.

KVSRoyal · 2026-03-17T18:46:26Z

@claude You are a principal engineer specialized in https://player-ui.github.io/next/ who follows best practices. Review this PR. Please follow these additional guidelines:

Use simple tenses where possible. Simple verb tenses (past, present, and future) keep things concise and people process simpler tenses faster than complex ones.
Keep words shorter than 9 characters, where possible. Technical terms are okay. Studies show that people are more likely to skip longer words.
Keep sentences shorter than 20 words. Longer sentences are harder to follow.
Use mermaid diagrams, tables, etc. where it will help illustrate a point.

claude · 2026-03-17T18:46:45Z

Claude finished @KVSRoyal's task in 2m 2s —— View job

PR Review: ExternalAction Handler Allows Overrides

Summary

Good work overall. The registry-based approach is clean and follows existing Player patterns. Found a few bugs and design concerns below.

Bugs

1. Logger set too late in `Registry` constructor

core/partial-match-registry/src/index.ts:48

constructor(initialSet?: Array<[any, V]>, logger?: Logger | undefined) {
  initialSet?.forEach(([match, value]) => {
    this.set(match, value); // ❌ logger is undefined here
  });
  this.logger = logger;     // too late
}

this.logger must be set before calling this.set. If initialSet has two entries with the same key, the debug log never fires.

Fix this →

2. Mixed test runner imports

plugins/external-action/core/src/__tests__/index.test.ts:5

import { test, expect, vitest } from "vitest";
import { describe } from "node:test"; // ❌ wrong runner

describe comes from node:test but all other helpers come from vitest. This will likely cause issues. Change to:

import { test, expect, vitest, describe } from "vitest";

Fix this →

Design Concerns

3. Symbol namespace may collide

plugins/external-action/core/src/symbols.ts:1

export const ExternalActionPluginSymbol = Symbol.for("ExternalActionPlugin");

Symbol.for is global. Any other package using "ExternalActionPlugin" will share this symbol. Use a namespaced key:

Symbol.for("@player-ui/ExternalActionPlugin")

Fix this →

4. `registry` is `private` but accessed cross-instance

plugins/external-action/core/src/index.ts:152

this.registry = existing.registry; // accessing private field on another instance

TypeScript allows private access within the same class—across instances. But this is a code smell. The reviewer also flagged this. Making registry public (or readonly) lets you drop the isFirstPluginInstance flag and simplify createRegistry. Here's the flow:

flowchart TD
  A[apply called] --> B{findPlugin returns existing?}
  B -- No --> C[Create new Registry]
  B -- Yes --> D[Use existing.registry]
  C --> E[Tap flow hooks]
  D --> F[Skip tap]

With a public registry, you can store it once and drop the isFirstPluginInstance field entirely.

5. Optional chaining on `registry` is misleading

plugins/external-action/core/src/index.ts:173

this.registry?.set(state, handler); // registry is always set when this runs

registerHandlers is called after createRegistry. registry is always set at this point. Remove the ?. or make the type non-optional.

Specificity Logic

This is the core behavior change. Good to call it out explicitly:

Scenario	Behavior
Two entries, same key	Later one wins (replaces)
Two entries, different specificity	More specific always wins
Same specificity, different values	Latest registration wins

The set logic uses both matchers to check mutual equality. This is correct:

const existingIndex = this.store.findIndex(
  (entry) => entry.matcher(match) && matcher(entry.key),
);

This handles asymmetry in partial matching. Good approach.

Minor Nits

entry && entry.matcher(query) in get — the entry && check is unnecessary. store only holds RegistryIndex<V> objects. Remove for clarity.
Missing test: no handler registered for an external state. What happens? The handler is undefined, so handler?.(...) returns undefined. No transition. This should be a test case.
The Registry constructor logger param has redundant | undefined — TypeScript adds that for optional params.

What's Good

The registry-based API is clean and follows Player's AssetRegistry pattern.
Tests cover: specificity ordering, override behavior, async, error, and edge cases.
Docs are updated with examples and cross-links.
Debug logging on override is a good DX addition.
The shouldTransition helper is clear and reusable.

codecov · 2026-04-02T16:49:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (a0a817c) to head (079dd91).
⚠️ Report is 32 commits behind head on player-1-dot-zero.

Additional details and impacted files

@@           Coverage Diff            @@
##   player-1-dot-zero   #817   +/-   ##
========================================
========================================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov · 2026-04-02T16:58:41Z

Bundle Report

Changes will increase total bundle size by 63.5kB (1.13%) ⬆️. This is within the configured threshold ✅

Detailed changes

Bundle name	Size	Change
plugins/common-expressions/core	407.65kB	1.2kB (0.29%) ⬆️
plugins/check-path/core	421.8kB	1.2kB (0.28%) ⬆️
plugins/stage-revert-data/core	385.42kB	1.2kB (0.31%) ⬆️
plugins/auto-scroll/react	19.9kB	228 bytes (1.16%) ⬆️
plugins/external-action/core*	18.89kB	-938 bytes (-4.73%) ⬇️
plugins/async-node/core	459.46kB	1.2kB (0.26%) ⬆️
plugins/markdown/core	662.46kB	1.2kB (0.18%) ⬆️
core/partial-match-registry	32.68kB	4.6kB (16.37%) ⬆️
plugins/reference-assets/core	464.42kB	1.2kB (0.26%) ⬆️
plugins/beacon/core	403.4kB	1.2kB (0.3%) ⬆️
plugins/metrics/core	439.96kB	1.2kB (0.27%) ⬆️
plugins/common-types/core	481.98kB	1.2kB (0.25%) ⬆️
core/player	953.28kB	1.2kB (0.13%) ⬆️
plugins/external-state/core	47.65kB	47.65kB (100%) ⬆️

ℹ️ *Bundle size includes cached data from a previous commit

Affected Assets, Files, and Routes:

view changes for bundle: plugins/async-node/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`AsyncNodePlugin.native.js`	1.2kB	411.61kB	0.29%

view changes for bundle: plugins/common-expressions/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`CommonExpressionsPlugin.native.js`	1.2kB	386.41kB	0.31%

view changes for bundle: plugins/reference-assets/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`ReferenceAssetsPlugin.native.js`	1.2kB	434.52kB	0.28%

view changes for bundle: plugins/check-path/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`CheckPathPlugin.native.js`	1.2kB	392.32kB	0.31%

view changes for bundle: core/partial-match-registry

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`Registry.native.js`	1.2kB	19.99kB	6.36% ⚠️
`cjs/index.cjs`	1.13kB	5.38kB	26.72% ⚠️
`index.legacy-esm.js`	1.13kB	3.66kB	44.91% ⚠️
`index.mjs`	1.13kB	3.66kB	44.91% ⚠️

view changes for bundle: plugins/common-types/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`CommonTypesPlugin.native.js`	1.2kB	410.81kB	0.29%

view changes for bundle: plugins/stage-revert-data/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`StageRevertDataPlugin.native.js`	1.2kB	378.6kB	0.32%

view changes for bundle: plugins/markdown/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`MarkdownPlugin.native.js`	1.2kB	637.3kB	0.19%

view changes for bundle: plugins/beacon/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`BeaconPlugin.native.js`	1.2kB	388.93kB	0.31%

view changes for bundle: plugins/external-action/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`ExternalActionPlugin.native.js`	-641 bytes	12.49kB	-4.88%
`cjs/index.cjs`	-99 bytes	2.82kB	-3.4%
`index.legacy-esm.js`	-99 bytes	1.79kB	-5.23%
`index.mjs`	-99 bytes	1.79kB	-5.23%

view changes for bundle: plugins/metrics/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`MetricsPlugin.native.js`	1.2kB	407.62kB	0.29%

view changes for bundle: core/player

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`Player.native.js`	1.2kB	403.2kB	0.3%

view changes for bundle: plugins/auto-scroll/react

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`cjs/index.cjs`	76 bytes	8.06kB	0.95%
`index.legacy-esm.js`	76 bytes	5.92kB	1.3%
`index.mjs`	76 bytes	5.92kB	1.3%

KVSRoyal · 2026-04-06T22:42:40Z

@claude Review this PR

claude · 2026-04-06T22:42:55Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

KVSRoyal · 2026-04-06T22:44:51Z

/docs

intuit-svc · 2026-04-06T22:56:06Z

Docs Preview

A preview of your PR docs was deployed by CircleCI #34847 on Mon, 06 Apr 2026 22:55:51 GMT

📖 Docs (View site)

sugarmanz

Small comments regarding documentation, otherwise looks good!

sugarmanz · 2026-04-09T07:18:18Z

 ]
 ```

+### `ExternalActionPlugin` renamed to `ExternalStatePlugin` with new handler API


Can these be added to the PR # Release Notes section so they get picked up and added to the CHANGELOG.md?

Yeah, sure, adding. Although for the 1.0 stuff I think we've so far just been putting things in the migration docs file.

sugarmanz · 2026-04-09T07:20:21Z

+  ExternalStateHandler(
+    ref = "my-action",
+    handlerFunction = ExternalStateHandler.Function { state, options, transition ->
+      transition("Next")
+    }
+  )


Does the trailing lambda syntax work here?

ExternalStateHandler(ref = "my-action") { state, options, transition -> transition("Next") }

It does. Updated!

sugarmanz · 2026-04-09T07:22:33Z

+  ExternalStatePlugin(
+    ExternalStateHandler(
+      ref = "my-action",
+      handlerFunction = ExternalStateHandler.Function { state, options, transition ->


Ditto about using trailing lambda syntax as suggested implementation format if it works

spentacular · 2026-04-09T05:11:37Z

+
+    player.hooks.flowController.tap(this.name, (flowController) => {
+      flowController.hooks.flow.tap(this.name, (flow) => {
+        flow.hooks.transition.tap(this.name, (fromState, toNamedState) => {


We either need to port these over or add a followup to get rid of the setTimeout: https://github.com/player-ui/player/blob/main/plugins/external-action/core/src/index.ts#L39

Update! I also tried to remove the param stuff from the web docs.

Co-authored-by: Spencer Hamm <spentacular@gmail.com>

KVSRoyal force-pushed the external-action branch from c1f2faa to 4808f90 Compare March 13, 2026 18:20

KVSRoyal force-pushed the external-action branch from 4808f90 to bd30886 Compare March 13, 2026 21:20

KetanReddy reviewed Mar 13, 2026

View reviewed changes

KetanReddy mentioned this pull request Mar 13, 2026

Centralize Logging Defenition for Player Ecosystem. #819

Open

KVSRoyal force-pushed the external-action branch from c3be8cb to 2f4f1cf Compare March 17, 2026 17:38

KVSRoyal closed this Mar 17, 2026

KVSRoyal reopened this Mar 17, 2026

KVSRoyal force-pushed the external-action branch from 099a17e to b516643 Compare March 17, 2026 18:40

spentacular reviewed Mar 18, 2026

View reviewed changes

This was referenced Mar 23, 2026

ExternalActionPlugin & ExternalActionViewModifierPlugin use array of match-handler pairs #829

Merged

partial-match-registry does not traverse null values #830

Merged

JVM supports deterministic override API for ExternalActionPlugin #831

Merged

KVSRoyal force-pushed the external-action branch from d6688f0 to 4330417 Compare April 2, 2026 16:49

KVSRoyal changed the title ~~ExternalAction Handler allows overrides~~ ExternalStatePlugin allows overrides Apr 6, 2026

intuit-svc added a commit to player-ui/player-ui.github.io that referenced this pull request Apr 6, 2026

Deploying docs for player-ui/player#817

724b355

JunDangIntuit approved these changes Apr 7, 2026

View reviewed changes

sugarmanz reviewed Apr 9, 2026

View reviewed changes

spentacular reviewed Apr 9, 2026

View reviewed changes

KVSRoyal marked this pull request as ready for review April 9, 2026 16:33

KVSRoyal requested a review from a team as a code owner April 9, 2026 16:33

KVSRoyal requested review from a team as code owners April 9, 2026 16:33

KVSRoyal and others added 12 commits April 12, 2026 15:56

ExternalAction use Registry by "keys"

ba6477e

Address AI Pr comments

aea0ad6

Lint fix

a2973ba

Apply suggestion from @spentacular

9ece5a6

Co-authored-by: Spencer Hamm <spentacular@gmail.com>

Address PR review

d8bbbba

Use array of tuples for EA constructor

6a919ef

Fix lint

ee7f3e3

ios & android support new ExternalAction override API (#829)

b1bd5ba

Rename to ExternalState uniformly

16819c7

Require ref and align inits

bc5c540

Update migration docs

5375de4

Update External State Plugin docs

fafcd7a

KVSRoyal force-pushed the external-action branch from e99c423 to fafcd7a Compare April 12, 2026 22:01

Use trailing lambda for jvm eg, clean up docs

079dd91

spentacular approved these changes Apr 13, 2026

View reviewed changes

sugarmanz approved these changes Apr 14, 2026

View reviewed changes

KVSRoyal merged commit 661488a into player-1-dot-zero Apr 14, 2026
23 of 24 checks passed

KVSRoyal deleted the external-action branch April 14, 2026 20:42

Conversation

KVSRoyal commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Change Type (required)

Does your PR have any documentation updates?

Release Notes

ExternalActionPlugin renamed to ExternalStatePlugin with new handler API

Handler matching and specificity

Overriding handlers across plugins

Missing handlers

Uh oh!

KetanReddy commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KVSRoyal commented Mar 17, 2026

Uh oh!

KVSRoyal commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: ExternalAction Handler Allows Overrides

Summary

Bugs

1. Logger set too late in Registry constructor

2. Mixed test runner imports

Design Concerns

3. Symbol namespace may collide

4. registry is private but accessed cross-instance

5. Optional chaining on registry is misleading

Specificity Logic

Minor Nits

What's Good

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov Bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle Report

Affected Assets, Files, and Routes:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Uh oh!

KVSRoyal commented Apr 6, 2026

Uh oh!

claude Bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KVSRoyal commented Apr 6, 2026

Uh oh!

intuit-svc commented Apr 6, 2026

Docs Preview

📖 Docs (View site)

Uh oh!

KVSRoyal commented Mar 13, 2026 •

edited

Loading

`ExternalActionPlugin` renamed to `ExternalStatePlugin` with new handler API

KetanReddy commented Mar 13, 2026 •

edited

Loading

KVSRoyal commented Mar 17, 2026 •

edited

Loading

claude Bot commented Mar 17, 2026 •

edited

Loading

1. Logger set too late in `Registry` constructor

4. `registry` is `private` but accessed cross-instance

5. Optional chaining on `registry` is misleading

codecov Bot commented Apr 2, 2026 •

edited

Loading

codecov Bot commented Apr 2, 2026 •

edited

Loading

claude Bot commented Apr 6, 2026 •

edited

Loading