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

google closure compiler got error on leaking arguments checking #26

Open
dunght160387 opened this issue May 19, 2017 · 2 comments
Open

Comments

@dunght160387
Copy link
Contributor

In the file
https://github.com/MailOnline/VPAIDHTML5Client/blob/master/js/VPAIDAdUnit.js
line 65-69

...
var ariaty = IVPAIDAdUnit.prototype[method].length;
// TODO avoid leaking arguments
// https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#32-leaking-arguments
var args = Array.prototype.slice.call(arguments);
var callback = (ariaty === args.length) ? args.pop() : undefined;
...

if my application use google closure compiler in ADVANCED mode (and offcource i'm sure i marked this file as extern), the value of ariaty always is 0. This makes the handshakeVersion cannot do the callback, so that the timeout occurs, then my application will get fail.

So, do you have any recommendations to use closure compiler correctly without this error?
Or would you change to use another way to avoid leaking-arguments?

I see that the IVPAIDAdUnit.prototype[method] always is function () {}, so the call IVPAIDAdUnit.prototype[method].length always is 0. In case of handshakeVersion, the IVPAIDAdUnit.prototype[method] should be function (version, callback) {}

Thank you!

@dunght160387
Copy link
Contributor Author

Additional, i used these debug lines to log to console:

...
var ariaty = IVPAIDAdUnit.prototype[method].length;
// TODO avoid leaking arguments
// https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#32-leaking-arguments
var args = Array.prototype.slice.call(arguments);
var callback = (ariaty === args.length) ? args.pop() : undefined;

DEBUG && console.log('IVPAIDAdUnit.METHODS.forEach: prototype=' + IVPAIDAdUnit.prototype);
DEBUG && console.log('IVPAIDAdUnit.METHODS.forEach: fn method(' + method + ')=' + IVPAIDAdUnit.prototype[method]);
DEBUG && console.log('IVPAIDAdUnit.METHODS.forEach: args(7)=' + args);
DEBUG && console.log('IVPAIDAdUnit.METHODS.forEach: callback=' + callback);
DEBUG && console.log('IVPAIDAdUnit.METHODS.forEach: ariaty=' + ariaty + ', args.length=' + args.length);
...

@dunght160387
Copy link
Contributor Author

I have 2 commits here https://github.com/dunght160387/VPAIDHTML5Client/commits/patch-2.
Please review and merge if it's correct.
Thank you!

iq-dot pushed a commit to iq-dot/VPAIDHTML5Client that referenced this issue Jul 28, 2017
* Added 'related video' header above title

* Actually displaying related video header

* title showing on video when user is active

* minor css changes to header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant