Skip to content

Externalized perf testing#836

Open
brocollie08 wants to merge 11 commits intomainfrom
externalized-perf-testing
Open

Externalized perf testing#836
brocollie08 wants to merge 11 commits intomainfrom
externalized-perf-testing

Conversation

@brocollie08
Copy link
Copy Markdown
Contributor

@brocollie08 brocollie08 commented Apr 1, 2026

Currently, the plugin native bundles contain an entire copy of Player - this PR externalizes the Player from the native bundles, reducing size burden of the native bundles. Native plugins now need core player bundle to be loaded before in order to use.

This also makes the change to iOS HeadlessPlayer to load core Player right away, before any plugin instantiation.

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
  • N/A

Does your PR have any documentation updates?

  • Updated docs
  • No Update needed
  • Unable to update docs
📦 Published PR as canary version: 0.15.3--canary.836.34893

Try this version out locally by upgrading relevant packages to 0.15.3--canary.836.34893

@nancywu1
Copy link
Copy Markdown
Contributor

nancywu1 commented Apr 1, 2026

/canary

@brocollie08
Copy link
Copy Markdown
Contributor Author

/canary

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (2ce16b6) to head (e394038).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #836   +/-   ##
===========================
===========================

☔ 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.

intuit-svc added a commit to player-ui/player-ui.github.io that referenced this pull request Apr 2, 2026
@intuit-svc
Copy link
Copy Markdown
Contributor

intuit-svc commented Apr 2, 2026

Build Preview

Your PR was deployed by CircleCI #34893 on Tue, 07 Apr 2026 18:09:14 GMT with this version:

0.15.3--canary.836.34893

📖 Docs (View site)

@brocollie08 brocollie08 marked this pull request as ready for review April 2, 2026 20:56
@brocollie08 brocollie08 requested review from a team as code owners April 2, 2026 20:56
Comment on lines +100 to +103
protected fun releaseRuntime() {
runBlocking { runtime.scope.coroutineContext[Job]?.children?.forEach { it.join() } }
runtime.release()
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: name this so it's clear what we're doing when we call it, releaseWhenIdle maybe?

tsup_config(name = "tsup_config")
tsup_config(
name = "tsup_config",
native_bundle = NATIVE_BUNDLE,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This plugin also has a dependency on the beacon plugin, which means it'll have it's own copy too. Possible to configure this to accept some extra externals? Or should we just auto-populate the native TSUP config to resolve all workspace packages?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is Beacon's presence here as "given" as Player is for other plugins?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We'd need to apply it here if not found to be sure

@sugarmanz
Copy link
Copy Markdown
Member

sugarmanz commented Apr 2, 2026

Add some release notes and/or update the PR title to describe the resulting change/impact. I've got a feeling this could cause some issues for consumers.

If we're changing bundling strategy too, we should update our docs to say why things are external or change the template.

We should really just refactor module loading in general to be a bit more standardized

@KetanReddy
Copy link
Copy Markdown
Member

I've got a feeling this could cause some issues for consumers.

Do we want to consider this as part of 1.0 or post 1.0 if it could be breaking?

@sugarmanz
Copy link
Copy Markdown
Member

I've got a feeling this could cause some issues for consumers.

Do we want to consider this as part of 1.0 or post 1.0 if it could be breaking?

Not necessarily, as long as we test in a few different places (like in any internal test framework). Just want the changelog to clearly reflect what changed in case we need to understand what happened when releasing this.

intuit-svc added a commit to player-ui/player-ui.github.io that referenced this pull request Apr 7, 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.

5 participants