-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
externalize @player-ui/player from native bundles #483
base: main
Are you sure you want to change the base?
Conversation
/canary |
Can we do this as a part of the rule to minimize the config needed on the usage side? |
It would likely require pulling more than what I touch out of this repo, since we define the template in this repo too: |
Ah, sorry. I thought we had some base tsup config that was expanded in our rules. |
@@ -2,16 +2,22 @@ load("@rules_player//javascript:defs.bzl", "js_pipeline") | |||
load("@npm//:defs.bzl", "npm_link_all_packages") | |||
load("//tools:defs.bzl", "NATIVE_BUILD_DEPS", "tsup_config", "vitest_config") | |||
|
|||
# TODO: Would be nice to macro all the native things to avoid missing this setup (consequence is bundling Player in a plugin bundle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any estimate for how much effort this would require? If its not huge would it be worth it to just do it now?
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.
Change Type (required)
Indicate the type of change your pull request is:
patch
minor
major
Does your PR have any documentation updates?