Skip to content

Commit 5b4987d

Browse files
dgirardidgirardi
authored
Prebid core & PBS adapter: Feature tags and optional compilation of native 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
1 parent 583c80b commit 5b4987d

23 files changed

+436
-308
lines changed

‎.eslintrc.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ module.exports = {
2121
globals: {
2222
'$$PREBID_GLOBAL$$': false,
2323
'BROWSERSTACK_USERNAME': false,
24-
'BROWSERSTACK_KEY': false
24+
'BROWSERSTACK_KEY': false,
25+
'FEATURES': 'readonly',
2526
},
2627
// use babel as parser for fancy syntax
2728
parser: '@babel/eslint-parser',

‎features.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[
2+
"NATIVE"
3+
]

‎gulpHelpers.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,5 +206,11 @@ module.exports = {
206206
}
207207

208208
return options;
209-
}
209+
},
210+
getDisabledFeatures() {
211+
return (argv.disable || '')
212+
.split(',')
213+
.map((s) => s.trim())
214+
.filter((s) => s);
215+
},
210216
};

‎gulpfile.js

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ var connect = require('gulp-connect');
99
var webpack = require('webpack');
1010
var webpackStream = require('webpack-stream');
1111
var gulpClean = require('gulp-clean');
12-
var KarmaServer = require('karma').Server;
13-
var karmaConfMaker = require('./karma.conf.maker.js');
1412
var opens = require('opn');
1513
var webpackConfig = require('./webpack.conf.js');
1614
var helpers = require('./gulpHelpers.js');
@@ -32,7 +30,8 @@ var prebid = require('./package.json');
3230
var port = 9999;
3331
const INTEG_SERVER_HOST = argv.host ? argv.host : 'localhost';
3432
const INTEG_SERVER_PORT = 4444;
35-
const { spawn } = require('child_process');
33+
const { spawn, fork } = require('child_process');
34+
const TerserPlugin = require('terser-webpack-plugin');
3635

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

143-
function makeWebpackPkg() {
144-
var cloned = _.cloneDeep(webpackConfig);
142+
function makeWebpackPkg(extraConfig = {}) {
143+
var cloned = _.merge(_.cloneDeep(webpackConfig), extraConfig);
145144
if (!argv.sourceMaps) {
146145
delete cloned.devtool;
147146
}
148147

149-
var externalModules = helpers.getArgModules();
148+
return function buildBundle() {
149+
var externalModules = helpers.getArgModules();
150150

151-
const analyticsSources = helpers.getAnalyticsSources();
152-
const moduleSources = helpers.getModulePaths(externalModules);
151+
const analyticsSources = helpers.getAnalyticsSources();
152+
const moduleSources = helpers.getModulePaths(externalModules);
153153

154-
return gulp.src([].concat(moduleSources, analyticsSources, 'src/prebid.js'))
155-
.pipe(helpers.nameModules(externalModules))
156-
.pipe(webpackStream(cloned, webpack))
157-
.pipe(gulp.dest('build/dist'));
154+
return gulp.src([].concat(moduleSources, analyticsSources, 'src/prebid.js'))
155+
.pipe(helpers.nameModules(externalModules))
156+
.pipe(webpackStream(cloned, webpack))
157+
.pipe(gulp.dest('build/dist'));
158+
}
158159
}
159160

160161
function getModulesListToAddInBanner(modules) {
@@ -272,9 +273,11 @@ function bundle(dev, moduleArr) {
272273

273274
function testTaskMaker(options = {}) {
274275
['watch', 'e2e', 'file', 'browserstack', 'notest'].forEach(opt => {
275-
options[opt] = options[opt] || argv[opt];
276+
options[opt] = options.hasOwnProperty(opt) ? options[opt] : argv[opt];
276277
})
277278

279+
options.disableFeatures = options.disableFeatures || helpers.getDisabledFeatures();
280+
278281
return function test(done) {
279282
if (options.notest) {
280283
done();
@@ -295,14 +298,7 @@ function testTaskMaker(options = {}) {
295298
process.exit(1);
296299
});
297300
} else {
298-
var karmaConf = karmaConfMaker(false, options.browserstack, options.watch, options.file);
299-
300-
var browserOverride = helpers.parseBrowserArgs(argv);
301-
if (browserOverride.length > 0) {
302-
karmaConf.browsers = browserOverride;
303-
}
304-
305-
new KarmaServer(karmaConf, newKarmaCallback(done)).start();
301+
runKarma(options, done)
306302
}
307303
}
308304
}
@@ -329,25 +325,24 @@ function runWebdriver({file}) {
329325
return execa(wdioCmd, wdioOpts, { stdio: 'inherit' });
330326
}
331327

332-
function newKarmaCallback(done) {
333-
return function (exitCode) {
328+
function runKarma(options, done) {
329+
// the karma server appears to leak memory; starting it multiple times in a row will run out of heap
330+
// here we run it in a separate process to bypass the problem
331+
options = Object.assign({browsers: helpers.parseBrowserArgs(argv)}, options)
332+
const child = fork('./karmaRunner.js');
333+
child.on('exit', (exitCode) => {
334334
if (exitCode) {
335335
done(new Error('Karma tests failed with exit code ' + exitCode));
336-
if (argv.browserstack) {
337-
process.exit(exitCode);
338-
}
339336
} else {
340337
done();
341-
if (argv.browserstack) {
342-
process.exit(exitCode);
343-
}
344338
}
345-
}
339+
})
340+
child.send(options);
346341
}
347342

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

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

427422
gulp.task('build-bundle-dev', gulp.series(makeDevpackPkg, gulpBundle.bind(null, true)));
428-
gulp.task('build-bundle-prod', gulp.series(makeWebpackPkg, gulpBundle.bind(null, false)));
423+
gulp.task('build-bundle-prod', gulp.series(makeWebpackPkg(), gulpBundle.bind(null, false)));
424+
// build-bundle-verbose - prod bundle except names and comments are preserved. Use this to see the effects
425+
// of dead code elimination.
426+
gulp.task('build-bundle-verbose', gulp.series(makeWebpackPkg({
427+
optimization: {
428+
minimizer: [
429+
new TerserPlugin({
430+
parallel: true,
431+
terserOptions: {
432+
mangle: false,
433+
format: {
434+
comments: 'all'
435+
}
436+
},
437+
extractComments: false,
438+
}),
439+
],
440+
}
441+
}), gulpBundle.bind(null, false)));
429442

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

434448
gulp.task('test-coverage', gulp.series(clean, testCoverage));
435449
gulp.task(viewCoverage);

‎karma.conf.maker.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ var _ = require('lodash');
77
var webpackConf = require('./webpack.conf.js');
88
var karmaConstants = require('karma').constants;
99

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

@@ -22,7 +22,7 @@ function newWebpackConfig(codeCoverage) {
2222
.flatMap((r) => r.use)
2323
.filter((use) => use.loader === 'babel-loader')
2424
.forEach((use) => {
25-
use.options = babelConfig({test: true});
25+
use.options = babelConfig({test: true, disableFeatures});
2626
});
2727

2828
if (codeCoverage) {
@@ -117,8 +117,8 @@ function setBrowsers(karmaConf, browserstack) {
117117
}
118118
}
119119

120-
module.exports = function(codeCoverage, browserstack, watchMode, file) {
121-
var webpackConfig = newWebpackConfig(codeCoverage);
120+
module.exports = function(codeCoverage, browserstack, watchMode, file, disableFeatures) {
121+
var webpackConfig = newWebpackConfig(codeCoverage, disableFeatures);
122122
var plugins = newPluginsArray(browserstack);
123123

124124
var files = file ? ['test/test_deps.js', file] : ['test/test_index.js'];

‎karmaRunner.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
const karma = require('karma');
2+
const process = require('process');
3+
const karmaConfMaker = require('./karma.conf.maker.js');
4+
5+
process.on('message', function(options) {
6+
try {
7+
let cfg = karmaConfMaker(options.coverage, options.browserstack, options.watch, options.file, options.disableFeatures);
8+
if (options.browsers && options.browsers.length) {
9+
cfg.browsers = options.browsers;
10+
}
11+
if (options.oneBrowser) {
12+
cfg.browsers = [cfg.browsers.find((b) => b.toLowerCase().includes(options.oneBrowser.toLowerCase())) || cfg.browsers[0]]
13+
}
14+
cfg = karma.config.parseConfig(null, cfg);
15+
new karma.Server(cfg, (exitCode) => {
16+
process.exit(exitCode);
17+
}).start();
18+
} catch (e) {
19+
// eslint-disable-next-line
20+
console.error(e);
21+
process.exit(1);
22+
}
23+
});

‎modules/prebidServerBidAdapter/index.js

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -438,18 +438,19 @@ let nativeEventTrackerMethodMap = {
438438
js: 2
439439
};
440440

441-
// enable reverse lookup
442-
[
443-
nativeDataIdMap,
444-
nativeImgIdMap,
445-
nativeEventTrackerEventMap,
446-
nativeEventTrackerMethodMap
447-
].forEach(map => {
448-
Object.keys(map).forEach(key => {
449-
map[map[key]] = key;
441+
if (FEATURES.NATIVE) {
442+
// enable reverse lookup
443+
[
444+
nativeDataIdMap,
445+
nativeImgIdMap,
446+
nativeEventTrackerEventMap,
447+
nativeEventTrackerMethodMap
448+
].forEach(map => {
449+
Object.keys(map).forEach(key => {
450+
map[map[key]] = key;
451+
});
450452
});
451-
});
452-
453+
}
453454
/*
454455
* Protocol spec for OpenRTB endpoint
455456
* e.g., https://<prebid-server-url>/v1/openrtb2/auction
@@ -558,7 +559,7 @@ Object.assign(ORTB2.prototype, {
558559

559560
const nativeParams = adUnit.nativeParams;
560561
let nativeAssets;
561-
if (nativeParams) {
562+
if (FEATURES.NATIVE && nativeParams) {
562563
let idCounter = -1;
563564
try {
564565
nativeAssets = nativeAssetCache[impressionId] = Object.keys(nativeParams).reduce((assets, type) => {
@@ -683,7 +684,7 @@ Object.assign(ORTB2.prototype, {
683684
}
684685
}
685686

686-
if (nativeAssets) {
687+
if (FEATURES.NATIVE && nativeAssets) {
687688
try {
688689
mediaTypes['native'] = {
689690
request: JSON.stringify({
@@ -1036,7 +1037,7 @@ Object.assign(ORTB2.prototype, {
10361037

10371038
if (bid.adm) { bidObject.vastXml = bid.adm; }
10381039
if (!bidObject.vastUrl && bid.nurl) { bidObject.vastUrl = bid.nurl; }
1039-
} else if (deepAccess(bid, 'ext.prebid.type') === NATIVE) {
1040+
} else if (FEATURES.NATIVE && deepAccess(bid, 'ext.prebid.type') === NATIVE) {
10401041
bidObject.mediaType = NATIVE;
10411042
let adm;
10421043
if (typeof bid.adm === 'string') {

‎modules/sizeMappingV2.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export function checkAdUnitSetupHook(adUnits) {
129129
Verify that 'config.active' is a 'boolean'.
130130
If not, return 'false'.
131131
*/
132-
if (mediaType === 'native') {
132+
if (FEATURES.NATIVE && mediaType === 'native') {
133133
if (typeof config[propertyName] !== 'boolean') {
134134
logError(`Ad unit ${adUnitCode}: Invalid declaration of 'active' in 'mediaTypes.${mediaType}.sizeConfig[${index}]'. ${conditionalLogMessages[mediaType]}`);
135135
isValid = false;
@@ -206,7 +206,7 @@ export function checkAdUnitSetupHook(adUnits) {
206206
}
207207
}
208208

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

‎plugins/pbjsGlobals.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,20 @@
22
let t = require('@babel/core').types;
33
let prebid = require('../package.json');
44
const path = require('path');
5+
const allFeatures = new Set(require('../features.json'));
6+
7+
const FEATURES_GLOBAL = 'FEATURES';
8+
9+
function featureMap(disable = []) {
10+
disable = disable.map((s) => s.toUpperCase());
11+
disable.forEach((f) => {
12+
if (!allFeatures.has(f)) {
13+
throw new Error(`Unrecognized feature: ${f}`)
14+
}
15+
});
16+
disable = new Set(disable);
17+
return Object.fromEntries([...allFeatures.keys()].map((f) => [f, !disable.has(f)]));
18+
}
519

620
function getNpmVersion(version) {
721
try {
@@ -13,6 +27,7 @@ function getNpmVersion(version) {
1327

1428
module.exports = function(api, options) {
1529
const pbGlobal = options.globalVarName || prebid.globalVarName;
30+
const features = featureMap(options.disableFeatures);
1631
let replace = {
1732
'$prebid.version$': prebid.version,
1833
'$$PREBID_GLOBAL$$': pbGlobal,
@@ -91,6 +106,17 @@ module.exports = function(api, options) {
91106
}
92107
}
93108
});
109+
},
110+
MemberExpression(path) {
111+
if (
112+
t.isIdentifier(path.node.object) &&
113+
path.node.object.name === FEATURES_GLOBAL &&
114+
!path.scope.hasBinding(FEATURES_GLOBAL) &&
115+
t.isIdentifier(path.node.property) &&
116+
features.hasOwnProperty(path.node.property.name)
117+
) {
118+
path.replaceWith(t.booleanLiteral(features[path.node.property.name]));
119+
}
94120
}
95121
}
96122
};

‎src/adapterManager.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,9 @@ adapterManager.makeBidRequests = hook('sync', function (adUnits, auctionStart, a
211211
* @see {@link https://github.com/prebid/Prebid.js/issues/4149|Issue}
212212
*/
213213
events.emit(CONSTANTS.EVENTS.BEFORE_REQUEST_BIDS, adUnits);
214-
decorateAdUnitsWithNativeParams(adUnits);
214+
if (FEATURES.NATIVE) {
215+
decorateAdUnitsWithNativeParams(adUnits);
216+
}
215217
adUnits = setupAdUnitMediaTypes(adUnits, labels);
216218

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

@@ -439,7 +441,7 @@ adapterManager.registerBidAdapter = function (bidAdapter, bidderCode, {supported
439441
if (includes(supportedMediaTypes, 'video')) {
440442
adapterManager.videoAdapters.push(bidderCode);
441443
}
442-
if (includes(supportedMediaTypes, 'native')) {
444+
if (FEATURES.NATIVE && includes(supportedMediaTypes, 'native')) {
443445
nativeAdapters.push(bidderCode);
444446
}
445447
} else {

‎src/adapters/bidderFactory.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ export function isValid(adUnitCode, bid, {index = auctionManager.index} = {}) {
542542
return false;
543543
}
544544

545-
if (bid.mediaType === 'native' && !nativeBidIsValid(bid, {index})) {
545+
if (FEATURES.NATIVE && bid.mediaType === 'native' && !nativeBidIsValid(bid, {index})) {
546546
logError(errorMessage('Native bid missing some required properties.'));
547547
return false;
548548
}

0 commit comments

Comments
 (0)