userID and adpod modules being included in prebid-core

[snapwich]

Type of issue

bug

Description

The userId and adpod modules are being included in prebid-core:

This is happening because the modules are being referenced from other modules directly:
https://github.com/prebid/Prebid.js/blob/master/modules/freeWheelAdserverVideo.js#L10
https://github.com/prebid/Prebid.js/blob/master/modules/digiTrustIdSystem.js#L15

Modules should not import other modules, doing so breaks the convention we've established where each module is a separate webpack entry point and module dependencies are specified at build time with --modules.

If we want to revisit that convention and how it's implemented we can, but as it is this causes issues with the build system thinking the modules are now "shared code" and belong where shared code goes: in prebid-core and are growing all prebid.js builds by 50kb whether they use these modules or not.

@jaiminpanchal27 @idettman @bretg @mkendall07

[jaiminpanchal27]

I tried one approach to solve this in one of my PR here https://github.com/prebid/Prebid.js/pull/3787/files#diff-583ded51e69735951bd128c9066eca2fR31

I added a common folder inside modules which will be a place to keep all shared code for modules. Intent here is that for any publisher they should only specify what id system they want to use in --modules. userId module is the base of it and should be automatically imported by Prebid build system.

Let me know what do you think or if you have any other way to solve this problem

[goosemanjack]

The userId system was designed with each IdSystem operating as a submodule of the userId module. In order to avoid build breaks where an IdSystem submodule is included in the build, but the userId is not explicitly included the userId gets automatically pulled in. If you don't want this included you should be able to exclude the digiTrustIdSystem to remove this dependency. So to be clear, this is expected behavior.

[snapwich]

you should be able to exclude the digiTrustIdSystem to remove this dependency. So to be clear, this is expected behavior.

I'm not sure I follow. If the modules ends up in prebid-core (like they are doing) that means they are included in all prebid.js bundles whether the digiTrustIdSystem system is included or not. Sounds unexpected considering you just described a different expected behavior.

@jaiminpanchal27 I'm checking out your code and taking a look right now. My intuition is saying this is going require either some addition to prebid-core to allow modules to communicate dependencies with each other, or some webpack hack around the current module system (seems unlikely to work, but maybe). Also, before we can even come up with those solutions, the requirements of a submodule system need to exist somewhere. Do they?

[bretg]

@jaiminpanchal27 - agree with @snapwich here -- userId and sub-mods can't be included by default.

[goosemanjack]

@bretg That would actually be a fairly significant behavioral change to the existing build process. Per build instructions in Readme:

The standard build output contains all the available modules from within the modules folder.

In order to exclude these modules you must specifically identify the modules you do want to include. EX:

 $ gulp build --modules=openxBidAdapter,rubiconBidAdapter,sovrnBidAdapter

https://github.com/prebid/Prebid.js#user-content-build-optimization

[snapwich]

@goosemanjack the standard build output isn't meant for production use. It's for testing and development. To get an actual production build people either: download through the download page (which is essentially an abstraction over gulp build --modules=...), include prebid-core and modules individually through npm into their application, or build their own prebid.js bundle from command line using --modules=.....

I think submodules definitely have some uses, we just need to figure out a better way to do it. I'm brainstorming some ideas right now but am open to ideas.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.