Skip to content

Commit

Permalink
Prebid core & PBS adapter: Feature tags and optional compilation of n…
Browse files Browse the repository at this point in the history
…ative support (#8219)

* Upgrade webpack to 5; gulp build works

* Fix karma, except events

* Uniform access to events, import * or require (import * from 'events.js' / import events from 'events.js' return different objects, which is a problem for stubbing)

* Fix (?) adapters that use `this` inappropriately

* Update webpack-bundle-analyzer

* Fix warnings

* Enable tree shaking

* Set webpack mode 'none' (or else tests fail (!))

* Update coreJS version in babelrc - #7943

* Use babel to translate to commonjs only for unit tests; enable production mode

* Define feature flags at compile time - starting with just "NATIVE"

* Add build-bundle-verbose

* Run tests with all features disabled

* Fix build-bundle-verbose

* Tag native#nativeAdapters

* Merge master

* Add fsevents as optional dep

* Adjust syntax for node 12
  • Loading branch information
dgirardi authored Jun 17, 2022
1 parent 583c80b commit 5b4987d
Show file tree
Hide file tree
Showing 23 changed files with 436 additions and 308 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ module.exports = {
globals: {
'$$PREBID_GLOBAL$$': false,
'BROWSERSTACK_USERNAME': false,
'BROWSERSTACK_KEY': false
'BROWSERSTACK_KEY': false,
'FEATURES': 'readonly',
},
// use babel as parser for fancy syntax
parser: '@babel/eslint-parser',
Expand Down
3 changes: 3 additions & 0 deletions features.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[
"NATIVE"
]
8 changes: 7 additions & 1 deletion gulpHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,5 +206,11 @@ module.exports = {
}

return options;
}
},
getDisabledFeatures() {
return (argv.disable || '')
.split(',')
.map((s) => s.trim())
.filter((s) => s);
},
};
80 changes: 47 additions & 33 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ var connect = require('gulp-connect');
var webpack = require('webpack');
var webpackStream = require('webpack-stream');
var gulpClean = require('gulp-clean');
var KarmaServer = require('karma').Server;
var karmaConfMaker = require('./karma.conf.maker.js');
var opens = require('opn');
var webpackConfig = require('./webpack.conf.js');
var helpers = require('./gulpHelpers.js');
Expand All @@ -32,7 +30,8 @@ var prebid = require('./package.json');
var port = 9999;
const INTEG_SERVER_HOST = argv.host ? argv.host : 'localhost';
const INTEG_SERVER_PORT = 4444;
const { spawn } = require('child_process');
const { spawn, fork } = require('child_process');
const TerserPlugin = require('terser-webpack-plugin');

// these modules must be explicitly listed in --modules to be included in the build, won't be part of "all" modules
var explicitModules = [
Expand Down Expand Up @@ -140,21 +139,23 @@ function makeDevpackPkg() {
.pipe(connect.reload());
}

function makeWebpackPkg() {
var cloned = _.cloneDeep(webpackConfig);
function makeWebpackPkg(extraConfig = {}) {
var cloned = _.merge(_.cloneDeep(webpackConfig), extraConfig);
if (!argv.sourceMaps) {
delete cloned.devtool;
}

var externalModules = helpers.getArgModules();
return function buildBundle() {
var externalModules = helpers.getArgModules();

const analyticsSources = helpers.getAnalyticsSources();
const moduleSources = helpers.getModulePaths(externalModules);
const analyticsSources = helpers.getAnalyticsSources();
const moduleSources = helpers.getModulePaths(externalModules);

return gulp.src([].concat(moduleSources, analyticsSources, 'src/prebid.js'))
.pipe(helpers.nameModules(externalModules))
.pipe(webpackStream(cloned, webpack))
.pipe(gulp.dest('build/dist'));
return gulp.src([].concat(moduleSources, analyticsSources, 'src/prebid.js'))
.pipe(helpers.nameModules(externalModules))
.pipe(webpackStream(cloned, webpack))
.pipe(gulp.dest('build/dist'));
}
}

function getModulesListToAddInBanner(modules) {
Expand Down Expand Up @@ -272,9 +273,11 @@ function bundle(dev, moduleArr) {

function testTaskMaker(options = {}) {
['watch', 'e2e', 'file', 'browserstack', 'notest'].forEach(opt => {
options[opt] = options[opt] || argv[opt];
options[opt] = options.hasOwnProperty(opt) ? options[opt] : argv[opt];
})

options.disableFeatures = options.disableFeatures || helpers.getDisabledFeatures();

return function test(done) {
if (options.notest) {
done();
Expand All @@ -295,14 +298,7 @@ function testTaskMaker(options = {}) {
process.exit(1);
});
} else {
var karmaConf = karmaConfMaker(false, options.browserstack, options.watch, options.file);

var browserOverride = helpers.parseBrowserArgs(argv);
if (browserOverride.length > 0) {
karmaConf.browsers = browserOverride;
}

new KarmaServer(karmaConf, newKarmaCallback(done)).start();
runKarma(options, done)
}
}
}
Expand All @@ -329,25 +325,24 @@ function runWebdriver({file}) {
return execa(wdioCmd, wdioOpts, { stdio: 'inherit' });
}

function newKarmaCallback(done) {
return function (exitCode) {
function runKarma(options, done) {
// the karma server appears to leak memory; starting it multiple times in a row will run out of heap
// here we run it in a separate process to bypass the problem
options = Object.assign({browsers: helpers.parseBrowserArgs(argv)}, options)
const child = fork('./karmaRunner.js');
child.on('exit', (exitCode) => {
if (exitCode) {
done(new Error('Karma tests failed with exit code ' + exitCode));
if (argv.browserstack) {
process.exit(exitCode);
}
} else {
done();
if (argv.browserstack) {
process.exit(exitCode);
}
}
}
})
child.send(options);
}

// If --file "<path-to-test-file>" is given, the task will only run tests in the specified file.
function testCoverage(done) {
new KarmaServer(karmaConfMaker(true, false, false, argv.file), newKarmaCallback(done)).start();
runKarma({coverage: true, browserstack: false, watch: false, file: argv.file}, done);
}

function coveralls() { // 2nd arg is a dependency: 'test' must be finished
Expand Down Expand Up @@ -425,11 +420,30 @@ gulp.task(clean);
gulp.task(escapePostbidConfig);

gulp.task('build-bundle-dev', gulp.series(makeDevpackPkg, gulpBundle.bind(null, true)));
gulp.task('build-bundle-prod', gulp.series(makeWebpackPkg, gulpBundle.bind(null, false)));
gulp.task('build-bundle-prod', gulp.series(makeWebpackPkg(), gulpBundle.bind(null, false)));
// build-bundle-verbose - prod bundle except names and comments are preserved. Use this to see the effects
// of dead code elimination.
gulp.task('build-bundle-verbose', gulp.series(makeWebpackPkg({
optimization: {
minimizer: [
new TerserPlugin({
parallel: true,
terserOptions: {
mangle: false,
format: {
comments: 'all'
}
},
extractComments: false,
}),
],
}
}), gulpBundle.bind(null, false)));

// public tasks (dependencies are needed for each task since they can be ran on their own)
gulp.task('test-only', test);
gulp.task('test', gulp.series(clean, lint, 'test-only'));
gulp.task('test-all-features-disabled', testTaskMaker({disableFeatures: require('./features.json'), oneBrowser: 'chrome', watch: false}))
gulp.task('test', gulp.series(clean, lint, gulp.series('test-all-features-disabled', 'test-only')));

gulp.task('test-coverage', gulp.series(clean, testCoverage));
gulp.task(viewCoverage);
Expand Down
8 changes: 4 additions & 4 deletions karma.conf.maker.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var _ = require('lodash');
var webpackConf = require('./webpack.conf.js');
var karmaConstants = require('karma').constants;

function newWebpackConfig(codeCoverage) {
function newWebpackConfig(codeCoverage, disableFeatures) {
// Make a clone here because we plan on mutating this object, and don't want parallel tasks to trample each other.
var webpackConfig = _.cloneDeep(webpackConf);

Expand All @@ -22,7 +22,7 @@ function newWebpackConfig(codeCoverage) {
.flatMap((r) => r.use)
.filter((use) => use.loader === 'babel-loader')
.forEach((use) => {
use.options = babelConfig({test: true});
use.options = babelConfig({test: true, disableFeatures});
});

if (codeCoverage) {
Expand Down Expand Up @@ -117,8 +117,8 @@ function setBrowsers(karmaConf, browserstack) {
}
}

module.exports = function(codeCoverage, browserstack, watchMode, file) {
var webpackConfig = newWebpackConfig(codeCoverage);
module.exports = function(codeCoverage, browserstack, watchMode, file, disableFeatures) {
var webpackConfig = newWebpackConfig(codeCoverage, disableFeatures);
var plugins = newPluginsArray(browserstack);

var files = file ? ['test/test_deps.js', file] : ['test/test_index.js'];
Expand Down
23 changes: 23 additions & 0 deletions karmaRunner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const karma = require('karma');
const process = require('process');
const karmaConfMaker = require('./karma.conf.maker.js');

process.on('message', function(options) {
try {
let cfg = karmaConfMaker(options.coverage, options.browserstack, options.watch, options.file, options.disableFeatures);
if (options.browsers && options.browsers.length) {
cfg.browsers = options.browsers;
}
if (options.oneBrowser) {
cfg.browsers = [cfg.browsers.find((b) => b.toLowerCase().includes(options.oneBrowser.toLowerCase())) || cfg.browsers[0]]
}
cfg = karma.config.parseConfig(null, cfg);
new karma.Server(cfg, (exitCode) => {
process.exit(exitCode);
}).start();
} catch (e) {
// eslint-disable-next-line
console.error(e);
process.exit(1);
}
});
29 changes: 15 additions & 14 deletions modules/prebidServerBidAdapter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -438,18 +438,19 @@ let nativeEventTrackerMethodMap = {
js: 2
};

// enable reverse lookup
[
nativeDataIdMap,
nativeImgIdMap,
nativeEventTrackerEventMap,
nativeEventTrackerMethodMap
].forEach(map => {
Object.keys(map).forEach(key => {
map[map[key]] = key;
if (FEATURES.NATIVE) {
// enable reverse lookup
[
nativeDataIdMap,
nativeImgIdMap,
nativeEventTrackerEventMap,
nativeEventTrackerMethodMap
].forEach(map => {
Object.keys(map).forEach(key => {
map[map[key]] = key;
});
});
});

}
/*
* Protocol spec for OpenRTB endpoint
* e.g., https://<prebid-server-url>/v1/openrtb2/auction
Expand Down Expand Up @@ -558,7 +559,7 @@ Object.assign(ORTB2.prototype, {

const nativeParams = adUnit.nativeParams;
let nativeAssets;
if (nativeParams) {
if (FEATURES.NATIVE && nativeParams) {
let idCounter = -1;
try {
nativeAssets = nativeAssetCache[impressionId] = Object.keys(nativeParams).reduce((assets, type) => {
Expand Down Expand Up @@ -683,7 +684,7 @@ Object.assign(ORTB2.prototype, {
}
}

if (nativeAssets) {
if (FEATURES.NATIVE && nativeAssets) {
try {
mediaTypes['native'] = {
request: JSON.stringify({
Expand Down Expand Up @@ -1036,7 +1037,7 @@ Object.assign(ORTB2.prototype, {

if (bid.adm) { bidObject.vastXml = bid.adm; }
if (!bidObject.vastUrl && bid.nurl) { bidObject.vastUrl = bid.nurl; }
} else if (deepAccess(bid, 'ext.prebid.type') === NATIVE) {
} else if (FEATURES.NATIVE && deepAccess(bid, 'ext.prebid.type') === NATIVE) {
bidObject.mediaType = NATIVE;
let adm;
if (typeof bid.adm === 'string') {
Expand Down
4 changes: 2 additions & 2 deletions modules/sizeMappingV2.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export function checkAdUnitSetupHook(adUnits) {
Verify that 'config.active' is a 'boolean'.
If not, return 'false'.
*/
if (mediaType === 'native') {
if (FEATURES.NATIVE && mediaType === 'native') {
if (typeof config[propertyName] !== 'boolean') {
logError(`Ad unit ${adUnitCode}: Invalid declaration of 'active' in 'mediaTypes.${mediaType}.sizeConfig[${index}]'. ${conditionalLogMessages[mediaType]}`);
isValid = false;
Expand Down Expand Up @@ -206,7 +206,7 @@ export function checkAdUnitSetupHook(adUnits) {
}
}

if (mediaTypes.native) {
if (FEATURES.NATIVE && mediaTypes.native) {
// Apply the old native checks
validatedNative = validatedVideo ? adUnitSetupChecks.validateNativeMediaType(validatedVideo) : validatedBanner ? adUnitSetupChecks.validateNativeMediaType(validatedBanner) : adUnitSetupChecks.validateNativeMediaType(adUnit);

Expand Down
26 changes: 26 additions & 0 deletions plugins/pbjsGlobals.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@
let t = require('@babel/core').types;
let prebid = require('../package.json');
const path = require('path');
const allFeatures = new Set(require('../features.json'));

const FEATURES_GLOBAL = 'FEATURES';

function featureMap(disable = []) {
disable = disable.map((s) => s.toUpperCase());
disable.forEach((f) => {
if (!allFeatures.has(f)) {
throw new Error(`Unrecognized feature: ${f}`)
}
});
disable = new Set(disable);
return Object.fromEntries([...allFeatures.keys()].map((f) => [f, !disable.has(f)]));
}

function getNpmVersion(version) {
try {
Expand All @@ -13,6 +27,7 @@ function getNpmVersion(version) {

module.exports = function(api, options) {
const pbGlobal = options.globalVarName || prebid.globalVarName;
const features = featureMap(options.disableFeatures);
let replace = {
'$prebid.version$': prebid.version,
'$$PREBID_GLOBAL$$': pbGlobal,
Expand Down Expand Up @@ -91,6 +106,17 @@ module.exports = function(api, options) {
}
}
});
},
MemberExpression(path) {
if (
t.isIdentifier(path.node.object) &&
path.node.object.name === FEATURES_GLOBAL &&
!path.scope.hasBinding(FEATURES_GLOBAL) &&
t.isIdentifier(path.node.property) &&
features.hasOwnProperty(path.node.property.name)
) {
path.replaceWith(t.booleanLiteral(features[path.node.property.name]));
}
}
}
};
Expand Down
8 changes: 5 additions & 3 deletions src/adapterManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,9 @@ adapterManager.makeBidRequests = hook('sync', function (adUnits, auctionStart, a
* @see {@link https://github.com/prebid/Prebid.js/issues/4149|Issue}
*/
events.emit(CONSTANTS.EVENTS.BEFORE_REQUEST_BIDS, adUnits);
decorateAdUnitsWithNativeParams(adUnits);
if (FEATURES.NATIVE) {
decorateAdUnitsWithNativeParams(adUnits);
}
adUnits = setupAdUnitMediaTypes(adUnits, labels);

let {[PARTITIONS.CLIENT]: clientBidders, [PARTITIONS.SERVER]: serverBidders} = partitionBidders(adUnits, _s2sConfigs);
Expand Down Expand Up @@ -425,7 +427,7 @@ adapterManager.callBids = (adUnits, bidRequests, addBidResponse, doneCb, request
function getSupportedMediaTypes(bidderCode) {
let supportedMediaTypes = [];
if (includes(adapterManager.videoAdapters, bidderCode)) supportedMediaTypes.push('video');
if (includes(nativeAdapters, bidderCode)) supportedMediaTypes.push('native');
if (FEATURES.NATIVE && includes(nativeAdapters, bidderCode)) supportedMediaTypes.push('native');
return supportedMediaTypes;
}

Expand All @@ -439,7 +441,7 @@ adapterManager.registerBidAdapter = function (bidAdapter, bidderCode, {supported
if (includes(supportedMediaTypes, 'video')) {
adapterManager.videoAdapters.push(bidderCode);
}
if (includes(supportedMediaTypes, 'native')) {
if (FEATURES.NATIVE && includes(supportedMediaTypes, 'native')) {
nativeAdapters.push(bidderCode);
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/adapters/bidderFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ export function isValid(adUnitCode, bid, {index = auctionManager.index} = {}) {
return false;
}

if (bid.mediaType === 'native' && !nativeBidIsValid(bid, {index})) {
if (FEATURES.NATIVE && bid.mediaType === 'native' && !nativeBidIsValid(bid, {index})) {
logError(errorMessage('Native bid missing some required properties.'));
return false;
}
Expand Down
Loading

0 comments on commit 5b4987d

Please sign in to comment.