j2cl icon indicating copy to clipboard operation
j2cl copied to clipboard

System.getProperty doesn't seem to work

Open sgammon opened this issue 4 years ago • 10 comments

Describe the bug I'm trying to use System.getProperty to access a Closure --define=, provided via defs. I can't seem to get it to work no matter what I do.

To Reproduce Example.java:

package app;
import jsinterop.annotations.JsType;

@JsType
public class Config {
  public static String getAppVersion() {
    return System.getProperty("APP_VERSION", "default_java");
  }
}

module.js:

goog.module('app');

/**
 * Defines the app version.
 *
 * @public
 * @define {!string} APP_VERSION
 */
const version = goog.define('APP_VERSION', 'default_js');

exports = {
  version: version
};

config-test.js:

goog.setTestOnly();

const app = goog.require('app');
const Config = goog.require('app.Config');


testSuite({
  testFrameworkVersion() {
    // tests framework version from JS
    assert(!!app.version);
  },

  testCompareVersions() {
    // tests framework version from J2CL
    assertEquals(
        app.version,
        Config.getAppVersion());
  }
});

BUILD.bazel:

package(default_visibility = ["//visibility:public"])
load("@com_google_j2cl//build_defs:rules.bzl", "j2cl_library")
load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_test")

j2cl_library(
    name = "Config",
    srcs = ["Config.java"],
)

closure_js_test(
    name = name,
    srcs = "config-test.js",
    html = "config_test.html",
    deps = [
      "@io_bazel_rules_closure//closure/library:testing",
      ":Config",
    ],
    defs = [
        "--define=APP_VERSION=abc123",
    ],
)

(config_test.html omitted because it basically just includes the JS).

The build then fails with the following exception:

java.lang.NullPointerException: NAME APP_VERSION 20 [length: 41] [source_file: bazel-out/darwin-dbg/bin/java/app/Config.js.zip!/app/Config.impl.java.js]
	at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:895)
	at com.google.javascript.jscomp.RemoveUnusedCode.getVarForNameNode(RemoveUnusedCode.java:719)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseNameNode(RemoveUnusedCode.java:574)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseNode(RemoveUnusedCode.java:420)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseChildren(RemoveUnusedCode.java:1122)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseCall(RemoveUnusedCode.java:631)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseNode(RemoveUnusedCode.java:348)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseChildren(RemoveUnusedCode.java:1122)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseNode(RemoveUnusedCode.java:429)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseChildren(RemoveUnusedCode.java:1122)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseFunction(RemoveUnusedCode.java:1258)
	at com.google.javascript.jscomp.RemoveUnusedCode.access$1200(RemoveUnusedCode.java:93)
	at com.google.javascript.jscomp.RemoveUnusedCode$Continuation.apply(RemoveUnusedCode.java:1586)
	at com.google.javascript.jscomp.RemoveUnusedCode.traverseAndRemoveUnusedReferences(RemoveUnusedCode.java:269)
	at com.google.javascript.jscomp.RemoveUnusedCode.process(RemoveUnusedCode.java:250)
	at com.google.javascript.jscomp.PhaseOptimizer$NamedPass.process(PhaseOptimizer.java:317)
	at com.google.javascript.jscomp.PhaseOptimizer.process(PhaseOptimizer.java:232)
	at com.google.javascript.jscomp.Compiler.performOptimizations(Compiler.java:2418)
	at com.google.javascript.jscomp.Compiler.lambda$stage2Passes$1(Compiler.java:799)
	at com.google.javascript.jscomp.CompilerExecutor$2.call(CompilerExecutor.java:102)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)

Outputs

goog.module('app.Config$impl');

const j_l_Object = goog.require('java.lang.Object$impl');
const $Util = goog.require('nativebootstrap.Util$impl');

class Config extends j_l_Object {
 
 constructor() {
  Config.$clinit();
  super();
  this.$ctor__app_Config__();
 }
 
 $ctor__gust_Config__() {
  this.$ctor__java_lang_Object__();
 }
 /** @return {?string} */
 static getAppVersion() {
  Config.$clinit();
  return $Util.$getDefine("APP_VERSION", "default_java");
 }
 
 static $clinit() {
  Config.$clinit = () =>{};
  Config.$loadModules();
  j_l_Object.$clinit();
 }
 /** @return {boolean} */
 static $isInstance(/** ? */ instance) {
  return instance instanceof Config;
 }
 
 static $loadModules() {}
 
}
$Util.$setClassMetadata(Config, 'app.Config');

exports = Config; 
//# sourceMappingURL=Config.js.map

Bazel version 2.0.0

Expected behavior I would expect it not to error, firstly. Then, I would expect it to emit abc123 in place of the string in the define.

If I leave the define unspecified, the error disappears, but then, of course, the default string is used, which fails the test.

sgammon avatar Feb 02 '20 01:02 sgammon

I also wanted to say, I did my best to follow along with #1 and the other existing issues, but I found trouble understanding them or applying them to my situation. What I am asking is, what is the supported way of accessing Closure defines in J2CL?

sgammon avatar Feb 02 '20 01:02 sgammon

Example.java/Config.java needs a goog.require("APP_VERSION") (... or goog.require('app')? i'm not as good with how closure modules work) to be emitted somehow in its eventual Example.java.impl.js, so that closure's knows where to find that define for Config.java.impl.js. Try adding creating a new file, named either Example.native.js or Config.native.js, and including the line above.


Second guess, due to my lack of familiarity with the effects of modules and exports: you might need to actually ask for System.getProperty("app.version). I'm not 100% on this, since the goog.define mechanism seems to just need it to be mentioned, not directly referenced? In #1, I'm following the code in google/jsinterop-base, where we split a plain goog.provide (no module) out from the native.js+System.getProperty.

niloc132 avatar Feb 03 '20 17:02 niloc132

@niloc132 i've also tried injecting the value via a Closure JS file with a @define/goog.define pair, which i then consume via that symbol in J2CL. and it still does not work. but i will give it a try with .native.js.

it is unclear to me if a --define symbol is the same as a symbol defined with @define, and how those two can converge. once a --define is symbolicated, so to speak, with some module/global variable, i would think the J2CL import would work, but then again i don't know how the Util class depicted above plays into things.

it seems to me from reading the code that the Util class mentioned looks for an "object path" matching the define, which would imply a post-symbolication --define, such that only passing --define=some.path.to.be=true would not work unless that path was attached to some symbol. i don't know if my understanding here is correct or not but i'll report back if i figure it out.

sgammon avatar Feb 03 '20 22:02 sgammon

You need both .js and .native.js. The .native.js adds the "goog.require" to your transpiled .java output (since you can't write a java import for a js symbol), and the .js file includes the @define'd goog.define(). The value passed to --define is what is returned from the goog.define call - and what is read via System.getProperty (which is transpiled via jsinterop to a call to Util.$getDefine(), which is implemented with goog.getObjectByName()).

Other examples that might help: provide+define https://github.com/VueGWT/vue-gwt/blob/master/core/src/main/java/com/axellience/vuegwt/core/client/VueGWT.js require, to be merged with the output from a .java file: https://github.com/VueGWT/vue-gwt/blob/master/core/src/main/java/com/axellience/vuegwt/core/client/VueGWT.native.js System.getProperty in the .java file: https://github.com/VueGWT/vue-gwt/blob/master/core/src/main/java/com/axellience/vuegwt/core/client/VueGWT.java#L39 Note that in this case, the provided namespace/object isn't actually connected to the name of the property that is actually being defined - plain provide/require dependencies just mean "hey, that file has to come before me", and that can be enough to change global vars.

One more example, in gwt-dom, trying to provide the same legacy "experience" of telling which browser is running (in this case because blink/webkit's RTL setup is ... not as easy to work with as gecko): https://github.com/jnehlmeier/gwt-dom/blob/master/src/main/resources/org/gwtproject/dom/client/dom.js https://github.com/jnehlmeier/gwt-dom/blob/master/src/main/resources/org/gwtproject/dom/client/Element%24UserAgentHolder.native.js https://github.com/jnehlmeier/gwt-dom/blob/master/src/main/java/org/gwtproject/dom/client/Element.java#L53

niloc132 avatar Feb 03 '20 22:02 niloc132

Sorry didn't read the whole thread yet but taking a quick; seems like you are using goog.module. goog.module doesn't work with @define; pls use goog.provide instead.

gkdn avatar Feb 04 '20 01:02 gkdn

@gkdn / @niloc132 thank you for your help on this, but I still can't get it to work :(

i've updated my sample according to your directions. here are the changes I made

  1. i added Config.native.js:
goog.require('app');
  1. i amended the file module.js:
goog.provide('app');

/**
 * Defines a symbol for the app version.
 *
 * @public
 * @define {!string} app.version
 */
app.version = goog.define('APP_VERSION', 'alpha');
  1. and, unchanged in my Config.java:
@JsType
public class Config {
  /**
   * Retrieve the application version setting.
   *
   * @return Version assigned for the currently-running application.
   **/
  public static String getAppVersion() {
    return System.getProperty("app.version", "alpha");
  }
}
  1. i am building it with these effective targets:
closure_js_library(
  name = "ConfigModule",
  srcs = ["module.js"],
)

j2cl_library(
  name = "Config",
  srcs = [
    "Config.java",
    "Config.native.js",
  ],
  deps = [
    ":ConfigModule",
  ]
)

sgammon avatar Feb 04 '20 23:02 sgammon

Pls change app.version = goog.define('APP_VERSION', 'alpha'); to app.version = goog.define('app.version', 'alpha');

System.getProperty uses the name passed to goog.define.

Also you can simplify your setup like following:

j2cl_library(
  name = "Config",
  srcs = [
    "Config.java",
    "Config.native.js",
    "module.js",
  ],
  # or you could simply do a glob for *.java and *.js
)

It would be great to put a sample for others to follow as an example; instead of digging for examples from jre or jsnterop.

gkdn avatar Feb 04 '20 23:02 gkdn

I’ll give that a shot and mark up a PR as a sample. thank you again for your help

sgammon avatar Feb 05 '20 00:02 sgammon

https://github.com/google/j2cl/blob/master/docs/best-practices.md#custom-compile-time-code-stripping should be updated to reflect the way that this is expected to work.

niloc132 avatar Feb 19 '20 19:02 niloc132

Thinking about it; it is easy introduce a Bazel macro to streamline this.

gkdn avatar Jun 13 '20 19:06 gkdn