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

Modules #1177

Merged
merged 111 commits into from Jun 26, 2017

Conversation

@snapwich
Collaborator

snapwich commented May 4, 2017

Type of change

  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes

Description of change

The module system as described in #1086

For an example of converting an existing adapter to a module look at the adaption in file src/adapters/rubicon.js to modules/rubiconAdapter.js and tests from test/spec/adapters/rubicon_spec.js to test/spec/modules/rubiconAdapter_spec.js.

New express module was added as well. With the new express module and the conversion of the rubcion adapter to a module the build currently spits out 3 files: prebid.js, express.js, and rubiconAdapter.js. These can then being bundled for usage after gulp is run using gulp bundle --modules=express,rubiconAdapter. The --modules flag is optional if you just want to bundle all modules found.

@snapwich snapwich requested a review from mkendall07 May 4, 2017

@protonate protonate self-requested a review May 8, 2017

@protonate protonate assigned protonate and snapwich and unassigned protonate May 8, 2017

@snapwich

This comment has been minimized.

Collaborator

snapwich commented May 8, 2017

Will update this pull-request with external module support using require.resolve for npm support.

@protonate

As discussed let's incorporate rollup and plugins to achieve this functionality while preserving & standardizing on es6 module syntax, also to minimize the required changes to adapters, tests, etc.

@snapwich

This comment has been minimized.

Collaborator

snapwich commented May 17, 2017

I've added a commit (5d7ade4) showing an example of splitting modules only using webpack code-splitting. I didn't update the express module as we'll still need to figure out the best way to get config to modules, but the rubiconAdapter module has been updated.

@mkendall07 mkendall07 referenced this pull request May 23, 2017

Closed

Structure proposal #1222

@protonate protonate added this to the Prebid 0.25.0 milestone Jun 2, 2017

@snapwich

This comment has been minimized.

Collaborator

snapwich commented Jun 7, 2017

I've updated this pull-request with the rubicon, appnexus, and appnexusAst adapters converted to modules.

To test you can clone the repo and checkout the modules branch, yarn install, and then gulp which generates the built components. Follow that with gulp bundle which bundles all of the built modules (by default) into an individual file build/dist/bundle.js.

You can also use gulp bundle --modules=appnexusBidAdapter,appnexusAstBidAdapter to specify only certain modules to be included in the bundle.js file.

gulp build will do both the build step and the bundle step as a convenience and accepts the same --modules argument.

Besides searching for modules in the ./modules directory, modules are also sought in node_modules directory from within the repo, or node_modules directories in parent folders. You can specify external modules by name using the same --modules argument. e.g. gulp build --modules=someNpmPkg. These external modules are included in the build process. i.e. external modules are ran through the webpack build so they can import, use babel features, and be written as a regular module would be written from within the project.

Starting tomorrow I will begin the process of converting the rest of the adapters into modules.

Note: One final thing probably needed before being production ready is to create the webpack plugin to exclude the manifest and jsonp loading code from the webpack runtime. These bundles work fine as is implemented here, but excluding that code will reduce the bundle size.

@snapwich

This comment has been minimized.

Collaborator

snapwich commented Jun 7, 2017

Another thing to note: in order to support supportedMediaTypes that was handled by the adapter-loader, video-loader, and native-loader; I have added an additional parameter to adaptermanger.registerBidAdapter that supports a configuration object that includes supportedMediaTypes

@protonate

This comment has been minimized.

Collaborator

protonate commented Jun 7, 2017

@matthewlane @jaiminpanchal27 @dbemiller added all of you as reviewers, please have a look as this is working towards final and will need to be understood by all of us.

ready for re-review

@dbemiller

This comment has been minimized.

Contributor

dbemiller commented Jun 8, 2017

I looked this over... but I must admit, I think I'm too late to the party to give much serious feedback.

A build task which creates a bundle with only the adapters you need looks like a great contribution. The spec was also super clear and well-written.

I'm getting hung up on the claim: "we want dynamic runtime inclusion without async loading." The main benefit of for dynamic runtime inclusion which I can think of is the fact that you can download only the parts of the javascript which you need. Since it sounds like you still expect publishers to have downloaded it all, then what benefit is there to loading things at runtime?

On the other hand... I don't actually see anything in the diff which adds APIs to allow that. So am I missing something, or did that part of the spec get dropped?

The rest is mostly details. Will leave some comments on individual files about them.

@@ -0,0 +1,200 @@

This comment has been minimized.

@dbemiller

dbemiller Jun 8, 2017

Contributor

Could you leave comments at the top of new files describing (at a high level) what their purpose is?

It's helpful for a few reasons... but those are even more pronounced now that we're making Modules.

  • It helps newcomers get their bearings on the codebase
  • It helps users decide whether or not they should include the module (at a glance) in their bundle
  • It (theoretically) improves code quality over time, because some future engineer tasked with implementing "feature X" can tell at a glance which (if any) code makes sense in this module.
"gulp-header": "^1.7.1",
"gulp-if": "^2.0.2",
"gulp-jscs": "^3.0.2",

This comment has been minimized.

@dbemiller

dbemiller Jun 8, 2017

Contributor

This looks accidental... are you using jscs anywhere? I thought we got rid of that with the move to eslint.

This comment has been minimized.

@snapwich

snapwich Jun 8, 2017

Collaborator

I added the gulp-if but am not sure what gulp-jscss deal is. If it was removed I might have accidentally re-added it after a merge conflict, I can remove it if its no longer needed.

This comment has been minimized.

@dbemiller

dbemiller Jun 9, 2017

Contributor

yeah, it was removed here in #1111

@@ -0,0 +1,24 @@
function Adapter(code) {

This comment has been minimized.

@dbemiller

dbemiller Jun 8, 2017

Contributor

Does this replace src/adapters/adapter.js? I don't see that file being deleted... but they're duplicates.

This comment has been minimized.

@snapwich

snapwich Jun 8, 2017

Collaborator

yes, this replaces src/adpaters/adapter.js. I haven't removed the old file yet because I haven't finished converting all the adapters to modules yet, so the non-converted adapters still look for this file. As soon as I'm done converting the adapters to modules I'll make sure this is moved properly in git.

configHandlers[property] = [];
}
configHandlers[property].push(handler);
}

This comment has been minimized.

@dbemiller

dbemiller Jun 8, 2017

Contributor

What happens if getConfig is called after the last call to setConfig? If I were calling these APIs, I would still expect my callback to be executed.

Actually... a related problem: I would expect my callback to be executed exactly once. It doesn't affect your use-cases here... but looking at this file in isolation, I'd still consider it a bug.

Related: are unit tests coming soon?

@@ -22,6 +24,10 @@ const VIDEO_ENDPOINT = '//fastlane-adv.rubiconproject.com/v1/auction/video';
const TIMEOUT_BUFFER = 500;
var ACCOUNT_ID;
getConfig('rubiconAdapter', config => ACCOUNT_ID = config.accountId);

This comment has been minimized.

@dbemiller

dbemiller Jun 8, 2017

Contributor

this looks..... well, very dangerous. By setting a variable in an async callback, you've inherently created a race condition. This works (I would guess?) because a user will (should?) call setConfig sometime between the time that this code runs, and when they kick off an auction... but if someone tried to use ACCOUNT_ID in this file outside of those auction-related functions, you could have some really tricky bugs to track down.

Part of me wants to say "your adapter, your business :p" but.... given that you're prototyping an architecture here, I'm a bit concerned because I can't really see a way to do it safely.

It seems to me like it would have to look something like:

getConfig('myNamespace', config => {
  var property = config.myParam;
  module.exports = stuffWhichUses(property);
});

...which I'm pretty sure modules don't actually let you do.

Can you write an example of a very small module which reads a value from the config, and then exports something useful once the param has been read? Without assuming that callers follow that implicit import files, setConfig, callUsefulMethod order.

This comment has been minimized.

@snapwich

snapwich Jun 8, 2017

Collaborator

I don't know if I'd call it a race condition, but there is definitely a dependency problem. However, I didn't create it. The problem is inherent in the way we execute pbjs.que blocks and when module code is ran by webpack. I detailed this in a diagram I pasted to slack awhile back to describe our problem:
screen_shot_2017-05-31_at_1 09 58_pm_480

We effectively have a circular dependency based on the current architecture of Prebid.js. This isn't a new problem, it's just now more obvious because this is the first time someone has attempted to apply global config (config across adUnits and auctions) to an adapter (or in this case now, a module). Currently configuration can only come in through bid.params which makes this condition less obvious, but is also not an ideal solution for handling global config.

Using a callback was the solution I found to solve this. By registering a callback that sets the config it allows us to defer configuration until after setConfig is called. I think the dependency is intuitive (that you must setConfig for a module before you can use that config), so I'm not too worried about the implications.

I originally tried to solve the problem with promises, but then I ran into an actual race condition when the scheduled config callback wasn't called until after the auction had ran.

It's possible to not register the adapter to Prebid.js until the config has been received (the registration being the real export that Prebid cares about, module.exports is only used for testing), but then you're just swapping a set of config error messages, for a more cryptic and general error message. i.e. "Rubcion Adapter missing required config Account ID" vs "Rubicon Adapter was not found".

This comment has been minimized.

@dbemiller

dbemiller Jun 9, 2017

Contributor

Yeah, that makes sense. I also agree that there's an architectural problem with prebid right now, and your diagram distills it down well.

One obvious fix that jumps out at me is if we moved towards an API like requestBids(config) instead, and then implement write requestBids to forward the config through to any adapters. That would also move us one step closer to multiple auctions, since it's clear what the config for each particular auction is.

I know fixing all that is outside the scope of this PR... but I wonder if we could get things a little closer to that ideal, without too much effort?

For example, right now your modules look like:

getConfig();
adapter = makeAdapter(config);
adaptermanager.registerBidAdapter(adapter, ...);

Instead, we could do something like:

function adapterLoader(config) {
  return makeAdapter(config);
}
adaptermanager.registerBidAdapterLoader(adapterLoader, ...);

Then load the config inside registerBidAdapterLoader, and forward the call through to registerBidAdapter in the config callback.

Main benefits being:

  • The circular dependence stays within prebid-core, and out of the modules
  • Adapters would be easier to unit test (can send through mock configs)

Another benefit (disjoint from the first... so take it as an independent suggestion) is that you could namespace the global config. For example, if the bidder code is rubicon, then registerAdapterLoader function could decide (universally) that rubiconAdapter is where your config data goes. Then when it actually executes your load function, it can send that data right back to you.

Benefits there being:

  • Lower publisher learning curve (config naming conventions consistent across all adapters)
  • Adapters can't trample each others' config values accidentally
@snapwich

This comment has been minimized.

Collaborator

snapwich commented Jun 8, 2017

So am I missing something, or did that part of the spec get dropped?

@dbemiller I wrote a longer response, but for whatever reason github posted it twice and when I deleted the extra one github decided to delete both of them.... It might still be in your e-mail, but if not, here's the gist of it:

The requirement of dynamic runtime inclusion was modified in a follow-up meeting concerning modules that happened after the spec was written. It was stressed that in an effort to keep es6 module syntax we should get rid of the runtime requirement. We still want quick dynamic inclusion though. And by that I mean, concatenating built files together to include required functionality (a process which is fast and could be part of a server-side request), or including them individually like:

<script src="./prebid.js"></script>
<script src="./rubiconBidAdapter.js"></script>
<script src="./appnexusBidAdapter.js"></script>
<script>
pbjs.processQueue();
pbjs.que.push(function() {
  ... stuff here
});
</script>
@GLStephen

This comment has been minimized.

Contributor

GLStephen commented Jun 8, 2017

@snapwich The latter (individual scripts) approach would likely see benefits in maintenance and performance in an HTTP2 environment if managed properly. In terms of caching and config and in terms of delivering from a central CDN while also allowing modularity with a known, off the shelf for everyone, mechanism: HTML.

@snapwich

This comment has been minimized.

Collaborator

snapwich commented Jun 8, 2017

@GLStephen I think it's a feasible approach. However, I don't think it'll be used often due to the rampant use of tag managers within the ad space and the hard requirement that the prebid.js code (that contains the webpack runtime) be loaded first (so other modules have something to register with).

For that reason I think the server-side concatenation of the bundle will be more common. I think that is still a very useful addition to Prebid.js though. It'll allow us to do things like:

<script src="./prebid.js?modules=rubiconBidAdapter,appnexusBidAdapter"></script>

and get exact bundles of only the code we want on request.

@dbemiller

This comment has been minimized.

Contributor

dbemiller commented Jun 9, 2017

I agree that server-side or build-time concatenation generally makes much more sense than multiple script tags on the page.

If you do want to leave that pattern open as a possibility, then the relevant parts of each module in this code (e.g. the registerBidAdapter code) should be wrapped inside the cmd. Otherwise you'll be forcing people to load prebid-core synchronously.

But that's pretty convoluted and icky if loading individual modules is a non-goal.

@protonate protonate removed this from the Prebid 0.25.0 milestone Jun 14, 2017

@snapwich

This comment has been minimized.

Collaborator

snapwich commented Jun 15, 2017

Modules

New Features

With the introduction of the module system, a new step has been added in the build phase of Prebid.js. Whereas before there was one long-running step that used adapters.json and analytics.json as configuration input to determine the final contents of prebid.js, there is now one long-running step that builds ALL the adapters (now modules) into multiple file outputs and one optional quick step to combine these into the final bundle.js file that can be included in your page.

gulp bundle command

With the new module system, the long-running first step can be ran individually using the default gulp command. This will spit out all of the modules found in the ./modules directory as built .js files in build/dist. These files can be included separately in the browser if wanted like so

<script src="./prebid.js"></script>
<script src="./rubiconBidAdapter.js"></script>
<script src="./appnexusBidAdapter.js"></script>
<script>
pbjs.processQueue();
pbjs.que.push(function() {
  ... stuff here
});
</script>

If you would like a singular bundle.js javascript file that you include in the browser instead, you can then create that bundle using the new gulp bundle command. With no arguments supplied, all modules will be bundled in the outputed file. Using the --modules argument, a list of modules, or a modules file can be provided that will specify which modules to bundle in the final output.

Examples
gulp 
gulp bundle    # all modules included in `bundle.js`
gulp bundle --modules=rubiconBidAdapter,appnexusBidAdapter
gulp bundle --modules=modules.json   # where modules.json contains ["rubiconBidAdapter", "appnexusBidAdapter"] 

The bundle.js file could then be used as follows

<script src="./bundle.js"></script>
<script>
// pbjs.processQueue(); no longer leaded
pbjs.que.push(function() {
  ... stuff here
});
</script>

pbjs.processQueue()

The Prebid.js command queue is no longer ran automatically, it needs to be executed after all your desired modules have been included in the page. It can be ran manually as in the example above including the individual modules as script tags. But as a convenience, using the gulp bundle command will add the pbjs.processQueue() command for you to the end of the bundle.js.

External modules

The --modules argument to gulp, gulp build, and/or, gulp bundle allows you to specify external modules that are not necessarily in the ./modules folder but that could be found using reuqire.resolve. This means if you have an adapter that you have installed with npm into the node_modules folder called testAdapter, it can be included into your local build using gulp build --modules=testAdapter

Migration

Some of the information below will help people who want to continue building Prebid.js as they had in the past or for those that need to update a build process to include the new steps.

Name changes

Since modules are an abstract concept, the adapters have been renamed to more specifically describe their application. For the rubicon adapter: adapters/rubicon.js => modules/rubiconBidAdapter.js, for appnexus analytics adapter adapters/analytics/appnexus.js => modules/appnexusAnalyticsAdapter.js. To include these files in the bundle.js they must be specified by their new plain name (without the folder path or extension, examples below).

gulp build command

This command is now a convenience that both builds and bundles for those that would like to hide the additional step invovled in the new modules build. To make a ready-to-go bundle.js with the desired adapters from source you can build as follows:

gulp build --modules=rubiconBidAdapter,appnexusAnalyticsAdapter

adapters.json, and analytics.json

These files are no longer necessary in the build process. If you would like to continue using a file to specify your adapters, you can do so using the --modules argument to pass the file name of the file that contains your json-formatted list of modules.

Example

gulp build --modules=modules.json

modules.json containing ["rubiconBidAdapter", "appnexusAnalyticsAdapter"] would be an effective replacement for running the old gulp build with adapters.json containing ["rubicon"] and analytics.json containing ["appnexus"]. It's no longer necessary to specify whether an adapter is a video adapter, native adapter, etc.

@protonate

LGTM!

@mkendall07 mkendall07 merged commit f6449d6 into prebid:master Jun 26, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mkendall07 mkendall07 removed the in progress label Jun 26, 2017

@protonate protonate removed the needs review label Jun 27, 2017

@navneetpandey

This comment has been minimized.

Contributor

navneetpandey commented Jun 27, 2017

Please note that this document http://prebid.org/dev-docs/bidder-adaptor.html now contains many broken link.

@bretg

This comment has been minimized.

Contributor

bretg commented Jun 27, 2017

Thanks @navneetpandey - covered in prebid/prebid.github.io#275. May have missed some -- review welcome.

@jaiminpanchal27 jaiminpanchal27 referenced this pull request Jun 27, 2017

Merged

Add ucfunnel adapter #1263

2 of 9 tasks complete

@dbemiller dbemiller referenced this pull request Jun 27, 2017

Merged

Audience Network: Add support for video format #1292

1 of 8 tasks complete
@navneetpandey

This comment has been minimized.

Contributor

navneetpandey commented Jun 30, 2017

Seem consistent now @bretg . Thanks.

Yann-Pravo added a commit to Yann-Pravo/Prebid.js that referenced this pull request Jul 6, 2017

Modules (prebid#1177)
* initial implementation of module system

* express module

* conversion of rubicon adapter to module system

* use var instead of let in node scripts

* example of using modules for concatenation through webpack code-splitting

* fixes for webpack code-splitting

* changes including config through setConfig and getConfig callback

* external modules working, but node_modules in project still needs babel-laoder fix

* gulp watch bundle building

* fix for babel-loader node_modules exclusion and bundle only

* updated express module to work in new webpack module format

* gulp default, don't bundle

* updated rubicon adapter to latest

* added appnexus and appnexusAst to modules.  added support for native and video registering

* added logging for files getting bundled

* fixed bug in bundling external modules

* aardvark bid adapter converted to module

* upadte appnexus bid adpater names and get ride of mocha loader tests

* adblade bid adapter converted to module

* adblund bid adapter converted to module

* adbutler bid adapter converted to module

* adequant bid adapter converted to module

* adform bid adapter converted to module

* adkernel bid adapter converted to module

* admedia bid adapter converted to module

* admixer bid adapter converted to module

* fix pbjs global for tests so load order doesn't matter

* adsuppply bid adpater converted to module

* use $$PREBID_GLOBAL$$ rather than pbjs in test files

* adyoulike bid adapter converted to module

* fixed adapter location in some adapters

* aol bid adapter converted to module

* atomx bid adapter converted to module

* audienceNetwork bid adapter converted to module

* beachfront bid adapter converted to module

* bidfluence bid adapter converted to module

* brightcom bid adapter converted to module

* centro bid adapter converted to module

* rubicon spec file name changed

* conversant bid adapter converted to module

* criteo bid adapter converted to module

* districtmDMX bid adapter converted to module

* fidelity bid adapter converted to module

* getintent bid adapter converted to module

* gumgum bid adapter converted to module

* hiromedia bid adapter converted to module

* huddledmasses bid adapter converted to module

* indexExchange bid adapter converted to module

* inneractive bid adapter converted to module

* innity bid adapter converted to module

* jcm bid adapter converted to module

* komoona bid adapter converted to module

* kruxlink bid adapter converted to module

* lifestreet bid adapter converted to module

* mantis bid adapter converted to module

* memeglobal bid adapter converted to module

* nginad bid adapter converted to module

* openx bid adapter converted to module

* piximedia bid adapter converted to module

* prebidServer bid adapter converted to module

* pubgears bid adapter converted to module

* pubmatic bid adapter converted to module

* pulsepoint and pulsepointLite bid adapter converted to module

* quantcast bid adapter converted to module

* rhythmone bid adapter converted to module

* added adkernel alias to headbidding to adkernalBidAdapter module

* roxot bid adapter converted to module

* sekindoUM bid adapter converted to module

* remove baseAdapter

* serverbid bid adapter converted to module

* sharethrough bid adapter converted to module

* smartyads bid adapter converted to module

* update conversantBidAdapter to support video

* get rubiconBidAdapter latest

* move rubicon adapter to module properly

* sonobi bid adapter converted to module

* sovrn bid adapter converted to module

* springserve bid adpater converted to module

* stickyadstv bid adapter converted to module (and removed console logs)

* convert tapsense bid adapter to module

* thoughtleadr bid adapter converted to module

* trion bid adapter converted to module

* triplelift bid adapter converted to module

* twenga bid adapter converted to module

* underdogmedia bid adapter converted to module

* vertamedia bid adapter converted to module

* vertoz bid adapter converted to module

* wideorbit bid adapter converted to module

* widespace bid adapter converted to module

* xhb bid adapter converted to module

* yieldbot converted from bid adapter to module

* smartadserver bid adapter converted to module

* remove old adapter class in adapter folder

* converted analytics adapters to modules

* removed loaders from build

* added plugin to remove file manifest and jsonp code from webpack runtime

* cox bid adapter converted to module

* carambola bid adapter converted to module

* eplanning bid adapter converted to module

* --modules arg allows .json file as parameter

* removing config module for now

* don't bundle AnalyticsAdapter.js in common chunk

* unruly bid adapter converted to module

* fixed case-sensitivity in including webpack plugin

* can't use class keyword, updated to constructor function for plugin

* added jsdoc to express and new webpack plugin

* fixed sourcemaps for concat step in bundle

* use external sourcemap for bundle instead of inline

* rename prebid.js to prebid-core.js and main bundle outputs prebid.js now

* updated README.md to be module specific

* added filter to ignore hidden files in modules directory

@dbemiller dbemiller referenced this pull request Jul 12, 2017

Merged

Sharethrough Adapter - set bidderCode on invalid bid #1295

4 of 4 tasks complete

jbAdyoulike added a commit to jbAdyoulike/Prebid.js that referenced this pull request Sep 21, 2017

Modules (prebid#1177)
* initial implementation of module system

* express module

* conversion of rubicon adapter to module system

* use var instead of let in node scripts

* example of using modules for concatenation through webpack code-splitting

* fixes for webpack code-splitting

* changes including config through setConfig and getConfig callback

* external modules working, but node_modules in project still needs babel-laoder fix

* gulp watch bundle building

* fix for babel-loader node_modules exclusion and bundle only

* updated express module to work in new webpack module format

* gulp default, don't bundle

* updated rubicon adapter to latest

* added appnexus and appnexusAst to modules.  added support for native and video registering

* added logging for files getting bundled

* fixed bug in bundling external modules

* aardvark bid adapter converted to module

* upadte appnexus bid adpater names and get ride of mocha loader tests

* adblade bid adapter converted to module

* adblund bid adapter converted to module

* adbutler bid adapter converted to module

* adequant bid adapter converted to module

* adform bid adapter converted to module

* adkernel bid adapter converted to module

* admedia bid adapter converted to module

* admixer bid adapter converted to module

* fix pbjs global for tests so load order doesn't matter

* adsuppply bid adpater converted to module

* use $$PREBID_GLOBAL$$ rather than pbjs in test files

* adyoulike bid adapter converted to module

* fixed adapter location in some adapters

* aol bid adapter converted to module

* atomx bid adapter converted to module

* audienceNetwork bid adapter converted to module

* beachfront bid adapter converted to module

* bidfluence bid adapter converted to module

* brightcom bid adapter converted to module

* centro bid adapter converted to module

* rubicon spec file name changed

* conversant bid adapter converted to module

* criteo bid adapter converted to module

* districtmDMX bid adapter converted to module

* fidelity bid adapter converted to module

* getintent bid adapter converted to module

* gumgum bid adapter converted to module

* hiromedia bid adapter converted to module

* huddledmasses bid adapter converted to module

* indexExchange bid adapter converted to module

* inneractive bid adapter converted to module

* innity bid adapter converted to module

* jcm bid adapter converted to module

* komoona bid adapter converted to module

* kruxlink bid adapter converted to module

* lifestreet bid adapter converted to module

* mantis bid adapter converted to module

* memeglobal bid adapter converted to module

* nginad bid adapter converted to module

* openx bid adapter converted to module

* piximedia bid adapter converted to module

* prebidServer bid adapter converted to module

* pubgears bid adapter converted to module

* pubmatic bid adapter converted to module

* pulsepoint and pulsepointLite bid adapter converted to module

* quantcast bid adapter converted to module

* rhythmone bid adapter converted to module

* added adkernel alias to headbidding to adkernalBidAdapter module

* roxot bid adapter converted to module

* sekindoUM bid adapter converted to module

* remove baseAdapter

* serverbid bid adapter converted to module

* sharethrough bid adapter converted to module

* smartyads bid adapter converted to module

* update conversantBidAdapter to support video

* get rubiconBidAdapter latest

* move rubicon adapter to module properly

* sonobi bid adapter converted to module

* sovrn bid adapter converted to module

* springserve bid adpater converted to module

* stickyadstv bid adapter converted to module (and removed console logs)

* convert tapsense bid adapter to module

* thoughtleadr bid adapter converted to module

* trion bid adapter converted to module

* triplelift bid adapter converted to module

* twenga bid adapter converted to module

* underdogmedia bid adapter converted to module

* vertamedia bid adapter converted to module

* vertoz bid adapter converted to module

* wideorbit bid adapter converted to module

* widespace bid adapter converted to module

* xhb bid adapter converted to module

* yieldbot converted from bid adapter to module

* smartadserver bid adapter converted to module

* remove old adapter class in adapter folder

* converted analytics adapters to modules

* removed loaders from build

* added plugin to remove file manifest and jsonp code from webpack runtime

* cox bid adapter converted to module

* carambola bid adapter converted to module

* eplanning bid adapter converted to module

* --modules arg allows .json file as parameter

* removing config module for now

* don't bundle AnalyticsAdapter.js in common chunk

* unruly bid adapter converted to module

* fixed case-sensitivity in including webpack plugin

* can't use class keyword, updated to constructor function for plugin

* added jsdoc to express and new webpack plugin

* fixed sourcemaps for concat step in bundle

* use external sourcemap for bundle instead of inline

* rename prebid.js to prebid-core.js and main bundle outputs prebid.js now

* updated README.md to be module specific

* added filter to ignore hidden files in modules directory
@jeremiegirault

This comment has been minimized.

jeremiegirault commented Nov 2, 2017

Hi, as of this day, the RequireEnsureWithoutJsonp.js has no effect. The PR #1785 references the chunk function name for some reason, but I don't think it works as it should. Worth fixing ?

@snapwich

This comment has been minimized.

Collaborator

snapwich commented Nov 8, 2017

@jeremiegirault what do you mean, it has no effect? I just pulled the latest and built prebid and it still seems to be successfully stubbing the jsonp loader code.

@jeremiegirault

This comment has been minimized.

jeremiegirault commented Nov 8, 2017

Hi, In my tests I had troubles and the plugin was not active indeed. But after using the good tag/master looking at the generated code it indeed seem to replace the requireEnsure function :

/******/ 	__webpack_require__.e = function requireEnsure(chunkId) {
/******/ 		if(installedChunks[chunkId] === 0)
/******/ 		  return callback.call(null, __webpack_require__);
/******/ 		else
/******/ 		  console.error('webpack chunk not found and jsonp disabled');
/******/ 	};

However callback does not exists. Indeed the prototype of requireEnsure used to be function requireEnsure(chunkId, callback). Now it returns a Promise (probably due to some internal webpack changes?).

Looking at the source without the plugin :

/******/ 	__webpack_require__.e = function requireEnsure(chunkId) {
/******/ 		var installedChunkData = installedChunks[chunkId];
/******/ 		if(installedChunkData === 0) {
/******/ 			return new Promise(function(resolve) { resolve(); });
/******/ 		}
/******/

...

So I question the usefulness of this webpack plugin which seem outdated in the context ?

@snapwich snapwich referenced this pull request Nov 8, 2017

Merged

just remove require.ensure entirely #1816

1 of 1 task complete
@snapwich

This comment has been minimized.

Collaborator

snapwich commented Nov 8, 2017

@jeremiegirault yeah that code is probably useless. It was an attempt to fail nicely if JSONP loading was attempted, but I don't think it's ever running. I added a pull-request to just remove that code block entirely.

dluxemburg added a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018

Modules (prebid#1177)
* initial implementation of module system

* express module

* conversion of rubicon adapter to module system

* use var instead of let in node scripts

* example of using modules for concatenation through webpack code-splitting

* fixes for webpack code-splitting

* changes including config through setConfig and getConfig callback

* external modules working, but node_modules in project still needs babel-laoder fix

* gulp watch bundle building

* fix for babel-loader node_modules exclusion and bundle only

* updated express module to work in new webpack module format

* gulp default, don't bundle

* updated rubicon adapter to latest

* added appnexus and appnexusAst to modules.  added support for native and video registering

* added logging for files getting bundled

* fixed bug in bundling external modules

* aardvark bid adapter converted to module

* upadte appnexus bid adpater names and get ride of mocha loader tests

* adblade bid adapter converted to module

* adblund bid adapter converted to module

* adbutler bid adapter converted to module

* adequant bid adapter converted to module

* adform bid adapter converted to module

* adkernel bid adapter converted to module

* admedia bid adapter converted to module

* admixer bid adapter converted to module

* fix pbjs global for tests so load order doesn't matter

* adsuppply bid adpater converted to module

* use $$PREBID_GLOBAL$$ rather than pbjs in test files

* adyoulike bid adapter converted to module

* fixed adapter location in some adapters

* aol bid adapter converted to module

* atomx bid adapter converted to module

* audienceNetwork bid adapter converted to module

* beachfront bid adapter converted to module

* bidfluence bid adapter converted to module

* brightcom bid adapter converted to module

* centro bid adapter converted to module

* rubicon spec file name changed

* conversant bid adapter converted to module

* criteo bid adapter converted to module

* districtmDMX bid adapter converted to module

* fidelity bid adapter converted to module

* getintent bid adapter converted to module

* gumgum bid adapter converted to module

* hiromedia bid adapter converted to module

* huddledmasses bid adapter converted to module

* indexExchange bid adapter converted to module

* inneractive bid adapter converted to module

* innity bid adapter converted to module

* jcm bid adapter converted to module

* komoona bid adapter converted to module

* kruxlink bid adapter converted to module

* lifestreet bid adapter converted to module

* mantis bid adapter converted to module

* memeglobal bid adapter converted to module

* nginad bid adapter converted to module

* openx bid adapter converted to module

* piximedia bid adapter converted to module

* prebidServer bid adapter converted to module

* pubgears bid adapter converted to module

* pubmatic bid adapter converted to module

* pulsepoint and pulsepointLite bid adapter converted to module

* quantcast bid adapter converted to module

* rhythmone bid adapter converted to module

* added adkernel alias to headbidding to adkernalBidAdapter module

* roxot bid adapter converted to module

* sekindoUM bid adapter converted to module

* remove baseAdapter

* serverbid bid adapter converted to module

* sharethrough bid adapter converted to module

* smartyads bid adapter converted to module

* update conversantBidAdapter to support video

* get rubiconBidAdapter latest

* move rubicon adapter to module properly

* sonobi bid adapter converted to module

* sovrn bid adapter converted to module

* springserve bid adpater converted to module

* stickyadstv bid adapter converted to module (and removed console logs)

* convert tapsense bid adapter to module

* thoughtleadr bid adapter converted to module

* trion bid adapter converted to module

* triplelift bid adapter converted to module

* twenga bid adapter converted to module

* underdogmedia bid adapter converted to module

* vertamedia bid adapter converted to module

* vertoz bid adapter converted to module

* wideorbit bid adapter converted to module

* widespace bid adapter converted to module

* xhb bid adapter converted to module

* yieldbot converted from bid adapter to module

* smartadserver bid adapter converted to module

* remove old adapter class in adapter folder

* converted analytics adapters to modules

* removed loaders from build

* added plugin to remove file manifest and jsonp code from webpack runtime

* cox bid adapter converted to module

* carambola bid adapter converted to module

* eplanning bid adapter converted to module

* --modules arg allows .json file as parameter

* removing config module for now

* don't bundle AnalyticsAdapter.js in common chunk

* unruly bid adapter converted to module

* fixed case-sensitivity in including webpack plugin

* can't use class keyword, updated to constructor function for plugin

* added jsdoc to express and new webpack plugin

* fixed sourcemaps for concat step in bundle

* use external sourcemap for bundle instead of inline

* rename prebid.js to prebid-core.js and main bundle outputs prebid.js now

* updated README.md to be module specific

* added filter to ignore hidden files in modules directory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment