closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

goog.reflect.objectProperty returning wrong name

Open AdobeScripter opened this issue 3 years ago • 4 comments

A disclaimer: I'm new to Closure Compiler and I know nothing about Closure Library, but I stumbled upon this StackOverflow question. It seemed like using goog.reflect.objectProperty would help me solve my problem, where I need to have a particular property name available as a string in the code. Why, you might ask? Well, because I have access to the setInterval function that can only accept string, so I need to know what to call it with.

I cannot include Closure Library in what I'm working at, but it seems it's enough to do var goog = { reflect: { objectProperty: function(name, obj) { return name; } } }; in the code to make it work. I'm not sure how it works, but I suppose this function is hard-coded in the compiler even without loading the library itself.

Enough with the background.

java -jar closure-compiler-v20201006.jar --js a.js -O ADVANCED --language_in ECMASCRIPT3 --language_out ECMASCRIPT3

This doesn't work with this code in a.js:

var goog = { reflect: { objectProperty: function(name, obj) { return name; } } };

var CoolNamespace = {
    fun: function() {
        alert('it worked');
    }
};

CoolNamespace.fun();
setTimeout(goog.reflect.objectProperty('CoolNamespace', this) + '.' + goog.reflect.objectProperty('fun', CoolNamespace) + '()', 1000);

because it returns:

var a={a:function(){alert("it worked")}};a.a();setTimeout("b.a()",1E3);

which is obviously wrong. We should get a.a() in the string.

What's even more funny to me is what happens when you delete this line from the source:

CoolNamespace.fun();

Then the resulting code is:

setTimeout("b.a()",1E3);

I kinda know what happens here, but I feel that this is still wrong: I might say I got a wrong thing out of goog.reflect.objectProperty call, because there is no b or a anywhere.

As I said, I'm very new to all this, but I hope this is real issue and not me misunderstanding something I don't fully comprehend yet.

AdobeScripter avatar Oct 24 '20 20:10 AdobeScripter

First, why pass a string to setTimeout? I don't know if you have some unusual requirement or actually are using some other function, but setTimeout(() => CoolNamespace.fun(), 1000); both seems more readable and doesn't require any Closure Compiler magic.

Second, the underlying issue is that Closure Compiler sometimes gets confused by referencing global variables off of the global object (like https://github.com/google/closure-compiler/issues/2162). So Closure Compiler doesn't connect goog.reflect.objectProperty('CoolNamespace', this) with the global variable CoolNamespace.

I'll leave this open so we can talk about this at the next bug triage meeting.

I suspect it's probably a bug but also there is probably an easier way to do what you want.

lauraharker avatar Oct 26 '20 18:10 lauraharker

As I wrote, I have setTimeout and setInterval that can only accept strings, no functions. I don't have a choice but to jump through hoops here.

AdobeScripter avatar Oct 26 '20 18:10 AdobeScripter

I'd suggest exporting the functions you want to call instead of using reflection (https://developers.google.com/closure/compiler/docs/externs-and-exports#purpose-of-exports).

That works around this issue with reflection & global variables, and might even allow Closure Compiler to better optimizer your code e.g. by replacing CoolNamespace.someFn with a single variable.

====

edit: either externs or exports might work for your use case. This is the more traditional way of making Closure Compiler not break references between compiled/uncompiled code.

Also, I guess your setInterval is from https://help.adobe.com/en_US/acrobat/acrobat_dc_sdk/2015/HTMLHelp/#t=Acro12_MasterBook%2FJS_API_AcroJS%2Fapp_methods.htm%23TOC_setTimeOutbc-35&rhtocid=_6_1_8_6_1_34?

lauraharker avatar Oct 27 '20 18:10 lauraharker

Yes, you guessed it. But externs/exports won't really work for me, since I want the code to be compiled. For now, this is my workaround:

/**
 * @param {*} obj
 * @return {string}
 */
var __objectToString__ = function(obj) {};

This is added to externs. Now, when I want to use setTimeout, I call it like this:

app.setTimeout(__objectToString__(CoolNamespace.fun) + '()', 1000);

And after compilation, when it gets replaced with something like __objectToString__(a.b), I can use regular expression to replace it with 'a.b'. It's ugly, I know, but I feel it's the best way to do it for me for now, if I want to have my code minified as much as possible.

AdobeScripter avatar Nov 08 '20 18:11 AdobeScripter