Prebid.js icon indicating copy to clipboard operation
Prebid.js copied to clipboard

Considering a "consumer" version of npm package

Open khatibda opened this issue 4 years ago • 11 comments

We just transitioned from building an offline bundle of prebid.js (using cloned repo) to using prebid via npm, and it's almost the best thing since sliced bread. But I have some questions / feedback on the process.

The current npm process feels geared towards "developer" and opposed to "consumers" (e.g. publishers). Bet let's walk through the details in case I'm missing something.

First, we've include prebid in our own package.json. Then in our app we have something like this:

`// import the pbjs global. the version pulled is determined in package.json import pbjs from 'prebid.js';

// register the desired modules/adapters import 'prebid.js/modules/aolBidAdapter'; import 'prebid.js/modules/appnexusBidAdapter'; import 'prebid.js/modules/consentManagement'; import 'prebid.js/modules/consentManagementUsp'; import 'prebid.js/modules/ixBidAdapter'; import 'prebid.js/modules/oneVideoBidAdapter'; import 'prebid.js/modules/openxBidAdapter'; import 'prebid.js/modules/prebidServerBidAdapter'; import 'prebid.js/modules/pubmaticBidAdapter'; import 'prebid.js/modules/rubiconBidAdapter'; import 'prebid.js/modules/spotxBidAdapter'; import 'prebid.js/modules/telariaBidAdapter'; import 'prebid.js/modules/tripleliftBidAdapter'; import 'prebid.js/modules/userId';

// not in prebid docs, but this seems required and is consistent with prior custom bundled code pbjs.processQueue();`

Minor feedback, but maybe your README.md could show the npm example with "import pbjs" instead of "import prebid", since you can define the global any way you want on import, so might as well be consistent with the default code publishers already likely have implemented in their app (pbjs.requestBids, etc).

Also, in our particular case, we weren't able to fetch bids until we called processQueue after import. This wasn't documented, but we tried it because we saw the same thing at the end of the prebid bundle we built offline (repo clone). I'm not sure why, but it worked once we did that.

Finally, in the case of a consumer, there doesn't need to be an expectation that people will be modifying the code base. In this case, can we just have access to pre-transpiled modules? Right now we have to include a webpack babel loader rule for prebid.js but this seem out of the ordinary. This is not how most of our other node package includes work. E.g. we just import React, and React is smart enough (looking at environment variables) to build as a dev option with more debug or as a prod min version. We don't have to transpile react ourselves. Same goes for most node modules, which is why people tend to ignore node modules folder in their babel loader rules.

Consumers should be able to include prebid core and then cherry pick which modules to include by specific imports, just as we have done above. For example, this is how we are able to import lodash/sortBy only, without pulling in the whole directory. In this manner, consumers can still include only what they need in their bundle. Separately there could be a modules.json interface or packages.json interface if you wanted to let publishers define inclusions or rules in a different way. But i think the cherry pick import could work.

I feel like if we were able to get past the raw transpiling step (and process queue), and just have people include prebid in package.json and then import as needed, this would open up a super fruitful and optimal approach for incorporating prebid into a publisher's stack.

Thoughts?

khatibda avatar May 21 '20 21:05 khatibda

cc @mkendall07 as per discussion

khatibda avatar May 21 '20 21:05 khatibda

I think the discussion in committee was around the popularity of this change to the build process. My assessment so far chatting with others is that we may have underestimated the popularity and this is actually quite useful. Will raise in the pub committee on June 4

patmmccann avatar Jun 01 '20 15:06 patmmccann

great, fingers crossed!

khatibda avatar Jun 02 '20 19:06 khatibda

Assigning to @patmmccann for visibility.

robertrmartinez avatar Jun 22 '20 20:06 robertrmartinez

is this actually a duplicate of your proposal on #2949 @snapwich ?

patmmccann avatar Jul 02 '20 23:07 patmmccann

@patmmccann yeah, it's something I've suggested in the past. There's some considerations to consider if we did this though, such as what target we compiled for, updates to the build/publish process, and then still making the source files available on npm as well for those that wanted to compile their own build.

@khatibda

// not in prebid docs, but this seems required and is consistent with prior custom bundled code pbjs.processQueue();`

This is in the README.md file. I wasn't aware consuming prebid from npm was documented on the docs site, if it is you should make a pull-request against that repo with the changes you're suggesting. I personally only documented this in this repo's README.md because I figured that's where most developers would look.

snapwich avatar Jul 23 '20 17:07 snapwich

yep i see it in the README.md file now, thx for pointing it out. all good on that front.

khatibda avatar Jul 30 '20 17:07 khatibda

As a note jsDelivr also added support for modules https://www.jsdelivr.com/esm which could be helpful here

jimaek avatar Nov 24 '20 15:11 jimaek

@khatibda do you have a handy link to the react NPM that we could review? I think we're open to doing some work here -- but want to make sure we don't clutter our NPM implementation on accident in the course of trying to set it up. Maybe we could discuss on the JS meeting this week?

gglas avatar Mar 01 '21 16:03 gglas

@gglas i'm assuming you mean a link to this, but lemme know if otherwise: https://www.npmjs.com/package/react

when is the JS meeting? i usually attend pub and video, but haven't attended JS before

khatibda avatar Mar 01 '21 18:03 khatibda

I plan to approach this by making both minified and unminified Prebid.js available in npm, so the "consumer" would do:

import 'prebid.js/dist/dev/prebid-core.js';     // or 'prebid.js/dist/min/prebid-core.js'
import 'prebid.js/dist/dev/someBidAdapter.js'   // or 'prebid.js/dist/min/someBidAdapter.js'
// etc

I think we should offer the choice between minified or not, rather than attempt to sniff that from the environment, as that's going to depend on the build tools in use - and unlike React we are not attempting to offer any additional integration with those.

dgirardi avatar Mar 11 '22 17:03 dgirardi

Until https://github.com/prebid/Prebid.js/issues/9333, I never realized that when pulled in as an npm dependency, it's possible to access all exports - 99% of which are not meant to be public. By allowing this we are inviting pubs to write logic that's very likely to break through updates.

I think we need a better approach than just making compilation results available; we should have an additional translation step that generates an npm package exporting only what is now available under the pbjs global.

dgirardi avatar Dec 13 '22 18:12 dgirardi