custom namespacing [cleaned up commit]

[baragona]

This is how I implemented: #115

I am totally open to other ways of implementing this feature. I just want this feature to exist.

This is the "obvious" way of doing namespacing - allow a configurable field in constants.json where the global variable name can be chosen.

Then everywhere pbjs was used needs to lookup what the global var is called.

[mkendall07]

Thanks for fixing the commit @ojotoxy

@protonate thoughts?

[protonate]

You don't need to lookup pbjs in every code file, doing so in ./src/prebid.js is sufficient and the rest of the code will load in the same context, so just the change to that file and in ./src/constants.json will do it. Additionally lookups on the window object are slow so we should only do this once, see here for example.

Otherwise this approach works great, thank you, please flatten commits and we'll merge it it.

[baragona]

So to clarify, this change in the appnexus adapter (and everywhere else) is not needed:

-  pbjs.handleAnCB = function (jptResponseObj) {
 +  window[CONSTANTS.PBJS_GLOBAL_VAR_NAME].handleAnCB = function (jptResponseObj) {

As the pbjs var used here is the one defined in src/prebid.js

It does seem necessary to look up the global var name at least once for most adapters, just to get the name for the JSONP callback.

I'll try this out and make a new PR with it cleaned up.

[protonate]

Ah, yes, in the case of JSONP callbacks it would be necessary.

[baragona]

Can you explain what you mean by "the rest of the code will load in the same context", I don't see how this is true in modules.

This snippet below from the appnexus adapter seems to break when the pbjs var is named something else. I don't see how it is expected that referring to pbjs here would use the variable defined in src/prebid.js. Modules have their own scope and won't see locals defined in other files. In src/prebid.js, "pbjs" is defined as a local.

  //expose the callback to the global object:
  pbjs.handleAnCB = function (jptResponseObj) {

Any thoughts on the best thing to do now?

[protonate]

Yes, you are right, sorry for the confusion.

There seems to be an issue with the TripleLife adapter when the dynamic name is not pbjs. The auction?callback= endpoint/param is defined twice, once with the dynamic name and once with pbjs.

I've looked at this more closely now and am thinking we shouldn't burden the dev community with the use case of supporting a non-standard syntax (e.g. window[CONSTANTS.PBJS_GLOBAL_VAR_NAME] for dynamic global name. It's much less of an issue in core code but even there it would be ideal to resolve this at build time so code runs as fast as possible (no global object property lookups) as well as optimization of code file size (the window[CONSTANTS.PBJS_GLOBAL_VAR_NAME] dynamic global accessor syntax cannot be uglified very well, and not at all in JSONP callbacks where it's most likely a string).

We could tokenize the global with %%PBJS_GLOBAL%% and replace it with a dynamic name during build time, such as we do with adapterLoader.js. But this presents an issue with dev time, IDEs and static analysis.

I'm experimenting with using a Symbol and will put up a PR for consideration if that works out.

[baragona]

Realistically, I think a global replacement of the token is not a bad idea. It could be done as the last compilation step. As long as the name used is very unique, it doesn't even need to surrounded with %%, thus not breaking dev tools.

The source code could refer to a variable like PbjsGlobal (unlikely to ever conflict with anything) and a compilation step could transform that to pbjs or anything the user wants.

[protonate]

Sounds good, want to update the PR with that approach?

Following the adapterLoader approach we can set the dynamic name in package.json and use blockLoader to replace the PbjsGlobal placeholder

[mkendall07]

Duplicate #293