Skip to content
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

Intent to implement: video disable feature #9421

Closed
mbcrute opened this issue Jan 13, 2023 · 6 comments
Closed

Intent to implement: video disable feature #9421

mbcrute opened this issue Jan 13, 2023 · 6 comments
Labels

Comments

@mbcrute
Copy link
Contributor

mbcrute commented Jan 13, 2023

Type of issue

Intent to implement, optimization

Description

Currently Prebid supports excluding code supporting the native media type at compile time via the --disable command line option of the Gulp build. Doing so trims several KB off of the resulting bundle size.

It should be possible to trim a few additional KB by extending this functionality to support excluding code supporting the video media type (e.g. --disable=NATIVE,VIDEO).

We have been doing some experimentation and analysis in our fork of Prebid and thus far have trimmed about 4KB.

We'd like to see if there's any interest in this feature from the maintainers and, if so, what's would be the best way to contribute (one PR? multiple PRs?).

@patmmccann
Copy link
Collaborator

This feature is absolutely welcomed 🙂 thank you

@spormeon
Copy link

how about a disable "banner" for those that only use video or native, is that even possible?

@GeneGenie
Copy link
Contributor

@spormeon i'd say biggest economy in core modules will be achieved with video disabling, but even more economy when all adapters will support feature flags, because video is connected with a lot code in adapters related to renderers.

@mbcrute is it possible to make disabling feature during bundling process rather than building? E.g i have a service that pre-builds all the modules, after that it only does bundling to avoid long delays. so our current fork has to be replicated in subfolders, to compile and bundle separately for those who don't want video.

@mbcrute
Copy link
Contributor Author

mbcrute commented Feb 8, 2023

Hello 👋 Just wanted to circle back on this to provide an update...

In our fork of Prebid.js we have extended the --disable feature of the build process to support excluding code specific to the video media type. We have implemented this in the core Prebid.js code as well as in several of the bid adapters that we use (AppNexus, OpenX, Index Exchange, TTD, Pubmatic, Rubicon). In doing so we have have reduced the resulting bundle size quite a bit.

We have been testing the changes to Prebid core on our website for the past week and are experiencing no issues so we feel confident that we're nearly ready to submit a PR for review.

My question is: what is the best way to do this? Should we open one large-ish pull request for all of the changes? Or should we open several smaller PRs - one for the changes to Prebid core and then one each for the changes to the bid adapters we've modified? Or is there some other approach that would be preferable?

@patmmccann

@patmmccann
Copy link
Collaborator

@mbcrute we've been seeing your changes come in and they are much appreciated! are there core changes pending or are we done with 9543?

@patmmccann
Copy link
Collaborator

please reopen if 9543 doesnt complete this, it looks like it does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

4 participants