POC: webpack-based automatic refactoring of bid adapters

[dgirardi]

Type of change

• Build related changes
• Other

Description of change

This adds build preprocessing that replaces certain import declaration in adapters' source files. The idea is to automatically refactor them to allow components like config or storageManager to always know who the bidder calling them is.

The strategy is:

• detect whether a source file is a bid adapter by looking for calls to registerBidder (pre-processing is also filtered just to files under modules/).
• if the source is a bid adapter, replace certain imports with an import for a factory function instead, and re-bind the imported name to a call to that factory function. For example, this:

  import { getStorageManager } from '../src/storageManager.js';

  is replaced with:

  import {storageManagerFactory} from "../src/adapters/bidderDepFactories.js";                             
  const getStorageManager = storageManagerFactory(bidderCode);  

Since import statements are entirely static this translation is pretty robust.

Caveats

• import * is not supported and will fail the build. it's not impossible to support but relatively complex and there are no adapters using it right now.
• The most complex part of this translation is determining the bidder code. I found two adapters that defined it in ways that are hard to evaluate statically; for now, I added the option of manually specifying it with a comment line. If the translator cannot evaluate the bidder code, the build will fail.
• It's possible to get around the translator by using import expressions (import(...)) or commonJS requires. I found two other adapters that used require and I replaced them with normal imports. If we decide to go ahead with this approach I believe we should try to block that either with eslint or in this translation step.
• I cannot find any decent documentation on how to make webpack (or babel, unclear who's unhappy) understand the source maps I generate; at the moment inspecting sources will only show the translation output.

Translation example

Compare the original:

Prebid.js/modules/adagioBidAdapter.js

Lines 1 to 30 in 51f9244

	import find from 'core-js-pure/features/array/find.js';
	import {
	isInteger, isArray, deepAccess, mergeDeep, logWarn, logInfo, logError, getWindowTop, getWindowSelf, generateUUID, _map,
	getDNT, parseUrl, getUniqueIdentifierStr, isNumber, cleanObj, isFn, inIframe, deepClone, getGptSlotInfoForAdUnitCode
	} from '../src/utils.js';
	import { config } from '../src/config.js';
	import {registerBidder} from '../src/adapters/bidderFactory.js';
	import { loadExternalScript } from '../src/adloader.js';
	import { verify } from 'criteo-direct-rsa-validate/build/verify.js';
	import { getStorageManager } from '../src/storageManager.js';
	import { getRefererInfo } from '../src/refererDetection.js';
	import { createEidsArray } from './userId/eids.js';
	import { BANNER, NATIVE, VIDEO } from '../src/mediaTypes.js';
	import { Renderer } from '../src/Renderer.js';
	import { OUTSTREAM } from '../src/video.js';
	const BIDDER_CODE = 'adagio';
	const LOG_PREFIX = 'Adagio:';
	const FEATURES_VERSION = '1';
	export const ENDPOINT = 'https://mp.4dex.io/prebid';
	const SUPPORTED_MEDIA_TYPES = [BANNER, NATIVE, VIDEO];
	const ADAGIO_TAG_URL = 'https://script.4dex.io/localstore.js';
	const ADAGIO_LOCALSTORAGE_KEY = 'adagioScript';
	const GVLID = 617;
	export const storage = getStorageManager(GVLID, 'adagio');
	export const RENDERER_URL = 'https://script.4dex.io/outstream-player.js';
	const MAX_SESS_DURATION = 30 * 60 * 1000;
	const ADAGIO_PUBKEY = 'AL16XT44Sfp+8SHVF1UdC7hydPSMVLMhsYknKDdwqq+0ToDSJrP0+Qh0ki9JJI2uYm/6VEYo8TJED9WfMkiJ4vf02CW3RvSWwc35bif2SK1L8Nn/GfFYr/2/GG/Rm0vUsv+vBHky6nuuYls20Og0HDhMgaOlXoQ/cxMuiy5QSktp';
	const ADAGIO_PUBKEY_E = 65537;
	const CURRENCY = 'USD';
	const DEFAULT_FLOOR = 0.1;

with the generated:

import find from "core-js-pure/features/array/find.js";
import {isInteger, isArray, deepAccess, mergeDeep, logWarn, logInfo, logError, getWindowTop, getWindowSelf, generateUUID, _map, getDNT, parseUrl, getUniqueIdentifierStr, isNumber, cleanObj, isFn, inIframe, deepClone, getGptSlotInfoForAdUnitCode} from "../src/utils.js";
import {registerBidder} from "../src/adapters/bidderFactory.js";
import {loadExternalScript} from "../src/adloader.js";
import {verify} from "criteo-direct-rsa-validate/build/verify.js";
import {getRefererInfo} from "../src/refererDetection.js";
import {createEidsArray} from "./userId/eids.js";
import {BANNER, NATIVE, VIDEO} from "../src/mediaTypes.js";
import {Renderer} from "../src/Renderer.js";
import {OUTSTREAM} from "../src/video.js";
import {configFactory, storageManagerFactory} from "../src/adapters/bidderDepFactories.js";
const config = configFactory("adagio");
const getStorageManager = storageManagerFactory("adagio");
const BIDDER_CODE = "adagio";
const LOG_PREFIX = "Adagio:";
const FEATURES_VERSION = "1";
export const ENDPOINT = "https://mp.4dex.io/prebid";
const SUPPORTED_MEDIA_TYPES = [BANNER, NATIVE, VIDEO];
const ADAGIO_TAG_URL = "https://script.4dex.io/localstore.js";
const ADAGIO_LOCALSTORAGE_KEY = "adagioScript";
const GVLID = 617;

[dgirardi]

CircleCI is failing to understand the ?? operator (used by a dependency I brought in). Tests are passing for me locally, I'll try to understand what is going on there.

[FilipStamenkovic]

You added more than 30K lines in package-lock.json file, is this really needed?
Source maps are not working properly (like you already mentioned in your comment), which would make pbjs harder to debug.

I do understand meaning of this POC (although, not entirely, I'm still not sure why you would fail when bidder imports config). You want to make sure that all bidders have passed their name in getStorageManager function?
But, why doing this at runtime? I think we can add some custom build (or lint) rule that says bid adapter must pass arguments in getStorageManager function. And if that doesn't happen then build (or lint) will fail.
What do you think?

[FilipStamenkovic]

CircleCI updates should be in separate PR

[FilipStamenkovic]

Why do we need this for config?
I understand for storageManager, but I don't get it for config.

How will this work for analyticsAdapters, rtbModules, idSystems, or any other module that is not bidder?

[dgirardi]

because config does things like this:

Prebid.js/src/config.js

Lines 293 to 301 in 95ab11e

	/**
	* Returns base config with bidder overrides (if there is currently a bidder)
	* @private
	*/
	function _getConfig() {
	if (currBidder && bidderConfig && isPlainObject(bidderConfig[currBidder])) {
	let currBidderConfig = bidderConfig[currBidder];
	const configTopicSet = new Set(Object.keys(config).concat(Object.keys(currBidderConfig)));

non-bidder modules are not affected by this, I don't know enough about them to know if it makes sense to do something similar to them. I tried to make this general enough that it should be relatively easy to extend for other use cases.

[FilipStamenkovic]

The whole purpose of this function is to throw an exception if some bid adapter calls getStorageManager without providing their bidder code?

Why not throwing an exception from the getStorageManager function itself? Is it because we would want to throw an exception only for bid adapters, but not for other modules?

[dgirardi]

it's just to prove that the whole thing works as a POC, the contents of this file are useless right now.

[dgirardi]

Also, getStorageManager is completely unaware of bidders - that's the point of this PR; we can't do anything* about bidders from there unless we change the interface to take in the bidder code. This is an attempt at a low-effort way to refactor the interface.

*Except for config's currentBidder "context", which does not work in some cases. I only consider bidders and not other modules because bidders are the only ones that have that special case already spread out through the codebase.

[dgirardi]

@FilipStamenkovic with a recent version of npm the first install will take ~10 minutes updating package.lock, it's not related to the code changes in here as far as I can tell. I can downgrade and hope that will reduce the diff, but then everyone else will have to go through those 10 minutes.

My hope with this pr is to replace the runWithBidder, currentBidder etc "magic" that is currently in config. Maybe I misunderstood what we mean by proof-of-concept; I don't expect or want this revision to be merged in, I'm just showing that we can use this approach to solve problems of the kind in #7280

[dgirardi]

Closing this in favor of #7992