Skip to content
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

Submodule system using hooks #3924

Merged
merged 21 commits into from
Jul 2, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4b2a8f1
prebid-core only contains src and a few node_modules
snapwich Jun 18, 2019
d036244
add back removed neverBundle to webpack build
snapwich Jun 18, 2019
3cd6523
add module and submodule hooks
snapwich Jun 19, 2019
76cba02
allow vargs for submodules for flexibility
snapwich Jun 20, 2019
47d576e
fix jsdoc type syntax
Jun 20, 2019
f158c76
Merge remote-tracking branch 'remote/master' into prebid-core-bundle-fix
Jun 24, 2019
9c0620e
updated id5 userid submodule for submodule bundle size duplication fix
Jun 24, 2019
6b9e976
add id5 userid submodule to .submodules
Jun 25, 2019
38b5727
fix opt out logic
Jun 25, 2019
b42a235
spelling fix to comment
Jun 25, 2019
207fea7
update to automatically include 'pubcommon' and 'unifiedid' (uncommen…
Jun 25, 2019
129abaf
Merge branch 'master' into prebid-core-bundle-fix
Jun 27, 2019
8398037
additional update to automatically include 'pubcommon' and 'unifiedid'
Jun 25, 2019
1bb74d8
additional update to automatically include 'pubcommon' and 'unifiedid'
Jun 25, 2019
f522add
Merge remote-tracking branch 'remote/prebid-core-bundle-fix' into pre…
Jun 27, 2019
45b2770
Merge branch 'master' of https://github.com/prebid/Prebid.js into pre…
Jun 27, 2019
b1e5401
merged differences from master
Jun 27, 2019
73d6f5b
adpod changes to support submodules
Jun 27, 2019
4aa8550
Merge branch 'prebid-core-bundle-fix' of github.com:prebid/Prebid.js …
Jun 27, 2019
058d448
fix --modules argument with .json to work correctly with submodules
snapwich Jun 27, 2019
5d3e818
fix to remove included and duplicated submodules
Jun 28, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion gulpHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ const MANIFEST = 'package.json';
const through = require('through2');
const _ = require('lodash');
const gutil = require('gulp-util');
const submodules = require('./modules/.submodules.json');

const MODULE_PATH = './modules';
const BUILD_PATH = './build/dist';
const DEV_PATH = './build/dev';
const ANALYTICS_PATH = '../analytics';


// get only subdirectories that contain package.json with 'main' property
function isModuleDirectory(filePath) {
try {
Expand Down Expand Up @@ -39,7 +41,18 @@ module.exports = {
.replace(/\/>/g, '\\/>');
},
getArgModules() {
var modules = (argv.modules || '').split(',').filter(module => !!module);
var modules = (argv.modules || '')
.split(',')
.filter(module => !!module);

Object.keys(submodules).forEach(parentModule => {
if (
!modules.includes(parentModule) &&
modules.some(module => submodules[parentModule].includes(module))
) {
modules.unshift(parentModule);
}
});

try {
if (modules.length === 1 && path.extname(modules[0]).toLowerCase() === '.json') {
Expand Down
7 changes: 7 additions & 0 deletions modules/.submodules.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"userId": [
"digiTrustIdSystem",
"pubCommonIdSystem",
"unifiedIdSystem"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think someone handled a merge improperly here. Duplicate and removed modules are showing up now...

Might want to check to make sure no other changes were accidentally reverted as well.

Copy link
Contributor

@idettman idettman Jun 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is supposed to be:

{
  "userId": [
    "digiTrustIdSystem",
    "id5IdSystem"
  ],
  "adpod": [
    "freeWheelAdserverVideo"
  ]
}

I checked the rest of the userId and it's releated files, and all look good. I'll commit a fix for the .submodules.json

]
}
4 changes: 2 additions & 2 deletions modules/digiTrustIdSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// import { config } from 'src/config';
import * as utils from '../src/utils'
import { ajax } from '../src/ajax';
import { attachIdSystem } from '../modules/userId';
import { submodule } from '../src/hook';
// import { getGlobal } from 'src/prebidGlobal';

/**
Expand Down Expand Up @@ -336,4 +336,4 @@ export const digiTrustIdSubmodule = {
_testInit: surfaceTestHook
};

attachIdSystem(digiTrustIdSubmodule);
submodule('userId', digiTrustIdSubmodule);
5 changes: 4 additions & 1 deletion modules/pubCommonIdSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* @requires module:modules/userId
*/

import * as utils from '../src/utils.js'
import * as utils from '../src/utils.js';
import { submodule } from '../src/hook';

/** @type {Submodule} */
export const pubCommonIdSubmodule = {
Expand Down Expand Up @@ -40,3 +41,5 @@ export const pubCommonIdSubmodule = {
return (pubcid) || utils.generateUUID();
}
};

submodule('userId', pubCommonIdSubmodule);
3 changes: 3 additions & 0 deletions modules/unifiedIdSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import * as utils from '../src/utils.js'
import {ajax} from '../src/ajax.js';
import { submodule } from '../src/hook';

/** @type {Submodule} */
export const unifiedIdSubmodule = {
Expand Down Expand Up @@ -53,3 +54,5 @@ export const unifiedIdSubmodule = {
}
}
};

submodule('userId', unifiedIdSubmodule);
11 changes: 4 additions & 7 deletions modules/userId.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
* @name Submodule#getId
* @param {SubmoduleParams} configParams
* @param {ConsentData} consentData
* @return {(Object|function} id data or a callback, the callback is called on the auction end event
* @return {(Object|function)} id data or a callback, the callback is called on the auction end event
*/

/**
* @function
* @summary decode a stored value for passing to bid requests
* @name Submodule#decode
* @param {Object|string} value
* @return {(Object|undefined}
* @return {(Object|undefined)}
*/

/**
Expand Down Expand Up @@ -73,9 +73,8 @@ import events from '../src/events.js';
import * as utils from '../src/utils.js';
import {getGlobal} from '../src/prebidGlobal.js';
import {gdprDataHandler} from '../src/adapterManager.js';
import {unifiedIdSubmodule} from './unifiedIdSystem.js';
import {pubCommonIdSubmodule} from './pubCommonIdSystem.js';
import CONSTANTS from '../src/constants.json';
import { module } from '../src/hook'

const MODULE_NAME = 'User ID';
const COOKIE = 'cookie';
Expand Down Expand Up @@ -419,6 +418,4 @@ export function init(config) {
// init config update listener to start the application
init(config);

// add submodules after init has been called
attachIdSystem(pubCommonIdSubmodule);
attachIdSystem(unifiedIdSubmodule);
module('userId', attachIdSystem);
13 changes: 13 additions & 0 deletions src/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,16 @@ export function setupBeforeHookFnOnce(baseFn, hookFn, priority = 15) {
baseFn.before(hookFn, priority);
}
}

export function module(name, install) {
hook('async', function (submodules) {
submodules.forEach(args => install(...args));
}, name)([]); // will be queued until hook.ready() called in pbjs.processQueue();
}

export function submodule(name, ...args) {
getHook(name).before((next, modules) => {
modules.push(args);
next(modules);
});
}
10 changes: 8 additions & 2 deletions webpack.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@ plugins.push( // this plugin must be last so it can be easily removed for karma
new webpack.optimize.CommonsChunkPlugin({
name: 'prebid',
filename: 'prebid-core.js',
minChunks: function(module, count) {
return !(count < 2 || neverBundle.indexOf(path.basename(module.resource)) !== -1)
minChunks: function(module) {
return (
(
module.context && module.context === path.resolve('./src') &&
!(module.resource && neverBundle.some(name => module.resource.includes(name)))
) ||
module.resource && module.resource.includes(path.resolve('./node_modules/core-js'))
);
}
})
);
Expand Down