Skip to content

feat: Add configurable "parent" prop to twitch-video#226

Open
ronalduQualabs wants to merge 1 commit intomuxinc:mainfrom
ronalduQualabs:feat/twitch-parent-prop
Open

feat: Add configurable "parent" prop to twitch-video#226
ronalduQualabs wants to merge 1 commit intomuxinc:mainfrom
ronalduQualabs:feat/twitch-parent-prop

Conversation

@ronalduQualabs
Copy link
Copy Markdown
Contributor

@ronalduQualabs ronalduQualabs commented Apr 9, 2026

Fixes #225

Adds a parent prop/attribute to <twitch-video-element> that accepts a string or array of domain strings, allowing embeds hosted across multiple domains to pass all required parent values to the Twitch iframe URL.

globalThis.location?.hostname is still included automatically, so existing usage is unaffected.


Note

Medium Risk
Changes how the Twitch iframe URL is constructed by appending one or more parent query params, which can affect embed loading if domains are misconfigured. Scope is small and mostly additive, but it touches the core URL serialization path used on every load.

Overview
Adds a configurable parent prop/attribute to twitch-video-element, allowing callers to provide one or more allowed parent domains for Twitch embeds.

Updates iframe URL generation to always include the current location.hostname plus any provided parent values (deduped) by appending multiple parent= query params, and triggers a reload when parent is set. Type definitions are updated to expose parent: string | string[] | null.

Reviewed by Cursor Bugbot for commit b6f1f68. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 9, 2026

@ronalduQualabs is attempting to deploy a commit to the Mux Team on Vercel.

A member of the Team first needs to authorize it.

@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Apr 9, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b6f1f68. Configure here.

set parent(value) {
this.#parent = value;
this.load();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setter lacks guard, can permanently stall loadComplete

Medium Severity

The parent setter unconditionally calls load() without checking whether the value actually changed. Unlike attributeChangedCallback which guards with if (oldValue === newValue) return, repeated sets of the same parent value (common in framework re-renders) cause load() to replace loadComplete with a new pending PublicPromise. Because the iframe URL hasn't changed, the iframe isn't recreated, the Twitch player never fires a new ready event, and loadComplete never resolves. This silently breaks all subsequent currentTime, volume, and muted sets, which depend on loadComplete.then(...).

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b6f1f68. Configure here.

Comment on lines +435 to +438
function serialize(props, parent) {
const params = new URLSearchParams(filterParams(props));
[].concat(parent).filter(Boolean).forEach(d => params.append('parent', d));
return String(params);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you pass parent inside props ( props.parent = parent) before calling serialize, this logic is implicitly done in filterParams

...props.config,
};

const parent = [...new Set([].concat(props.parent ?? []).concat(globalThis.location?.hostname ?? []))].filter(Boolean);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const parent = [...new Set([].concat(props.parent ?? []).concat(globalThis.location?.hostname ?? []))].filter(Boolean);
const parent = [...new Set([].concat(props.parent ?? []).concat(globalThis.location?.hostname ?? []))].filter(Boolean);
props.parent = parent;

Regarding previous comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And rolling back to

return ${EMBED_BASE}/?video=v${videoId}&${serialize(params)};

Comment on lines +135 to +138
set parent(value) {
this.#parent = value;
this.load();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Cursor wants this which makes sense to me

Suggested change
set parent(value) {
this.#parent = value;
this.load();
}
set parent(value) {
if (value === this.#parent) return;
this.#parent = value;
this.load();
}

Copy link
Copy Markdown
Contributor

@spuppo-mux spuppo-mux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments

@spuppo-mux spuppo-mux self-assigned this Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

<twitch-video> parent should be an array

2 participants