Skip to content

Commit

Permalink
Rework the backing machinery for System.getProperty to utilize our ow…
Browse files Browse the repository at this point in the history
…n registry

The existing system is brittle properties are read as global symbols when
uncompiled, but as `goog.define` flags when compiled. This is error-prone and
there's no clear linkage between the two.

To replace it, system properties now need to be explicitly registered. For now
we'll only be supporting registering properties that come from a `goog.define`
flag.

For backwards compatibility we'll fallback to `goog.getObjectByName` if a
property is not registered, but we will make this an error in the future.

PiperOrigin-RevId: 689188398
  • Loading branch information
kevinoconnor7 authored and copybara-github committed Oct 24, 2024
1 parent 1d230d1 commit 7a3f585
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 15 deletions.
21 changes: 18 additions & 3 deletions docs/best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,23 @@ production JavaScript code.
### Custom Compile-Time Code Stripping

You can implement your own configuration based stripping with
`System.getProperty()`. In the following example, the compiler will statically
evaluate the condition and remove the entire if/else control statement.
`System.getProperty()` paired with a `goog.define` property in JS. In the
following example, the compiler will statically evaluate the condition and
remove the entire if/else control statement.

```js
const jre = goog.require('jre');

// First declare the goog.define name. If you don't use a string type, it will
// be stringified when read in Java.
/** @define {string} */
const whatever = goog.define('some.define', 'NO');

// Add the define to the set of system properties. The name must match the
// goog.define name, and the value must be the result of the corresponding
// goog.define call.
jre.addSystemPropertyFromGoogDefine('some.define', whatever);
```

```java
if (System.getProperty("some.define") == "YES") {
Expand All @@ -221,7 +236,7 @@ if (System.getProperty("some.define") == "YES") {
}
```

```python
```build
js_binary(
name = "optimized_j2cl_app",
defs = ["--define=some.define=YES"] + J2CL_OPTIMIZED_DEFS,
Expand Down
67 changes: 67 additions & 0 deletions jre/java/java/lang/jre.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,37 +20,104 @@

goog.provide('jre');

/** @private @const {!Object<string, ?string>} */
jre.systemProperties_ = Object.create(null);

/**
* Adds a system property connected to goog.define.
* Properties being registered must have a name that matches the goog.define
* name and the value must be the result of the goog.define call. These values
* will then be accessible via System.getProperty in Java.
* @param {string} googDefineName
* @param {?string} googDefineValue
*/
jre.addSystemPropertyFromGoogDefine = function(
googDefineName, googDefineValue) {
// Values from goog.define are well-known to the compiler and will be
// statically substituted. Therefore this method can be a no-op when compiled.
if (!COMPILED) {
if (googDefineName in jre.systemProperties_) {
throw new Error(
`Attempting to redeclare system property '${googDefineName}'.`);
}
jre.systemProperties_[googDefineName] = googDefineValue;
}
};

/**
* Returns the value of a system property.
* @param {string} name
* @param {?string=} defaultValue
* @return {?string}
*/
jre.getSystemProperty = function(name, defaultValue = null) {
// For backwards compatibility, fallback to goog.getObjectByName if it's not
// in the registry.
const rv = jre.systemProperties_[name] ?? goog.getObjectByName(name);
return rv == null ? defaultValue : String(rv);
};

// Add core defines
jre.addSystemPropertyFromGoogDefine('COMPILED', String(COMPILED));
jre.addSystemPropertyFromGoogDefine('goog.DEBUG', String(goog.DEBUG));
jre.addSystemPropertyFromGoogDefine('goog.LOCALE', goog.LOCALE);
jre.addSystemPropertyFromGoogDefine(
'goog.TRUSTED_SITE', String(goog.TRUSTED_SITE));
jre.addSystemPropertyFromGoogDefine(
'goog.FEATURESET_YEAR', String(goog.FEATURESET_YEAR));

/** @define {string} */
jre.classMetadata = goog.define('jre.classMetadata', 'SIMPLE');
jre.addSystemPropertyFromGoogDefine('jre.classMetadata', jre.classMetadata);

/** @define {string} */
jre.checkedMode =
goog.define('jre.checkedMode', goog.DEBUG ? 'ENABLED' : 'DISABLED');
jre.addSystemPropertyFromGoogDefine('jre.checkedMode', jre.checkedMode);


goog.provide('jre.checks');

/** @define {string} */
jre.checks.checkLevel = goog.define('jre.checks.checkLevel', 'NORMAL');
jre.addSystemPropertyFromGoogDefine(
'jre.checks.checkLevel', jre.checks.checkLevel);

/** @define {string} */
jre.checks.bounds = goog.define('jre.checks.bounds', 'AUTO');
jre.addSystemPropertyFromGoogDefine('jre.checks.bounds', jre.checks.bounds);


/** @define {string} */
jre.checks.api = goog.define('jre.checks.api', 'AUTO');
jre.addSystemPropertyFromGoogDefine('jre.checks.api', jre.checks.api);

/** @define {string} */
jre.checks.numeric = goog.define('jre.checks.numeric', 'AUTO');
jre.addSystemPropertyFromGoogDefine('jre.checks.numeric', jre.checks.numeric);

/** @define {string} */
jre.checks.type = goog.define('jre.checks.type', 'AUTO');
jre.addSystemPropertyFromGoogDefine('jre.checks.type', jre.checks.type);

/** @define {string} */
jre.checks.critical = goog.define('jre.checks.critical', 'AUTO');
jre.addSystemPropertyFromGoogDefine('jre.checks.critical', jre.checks.critical);


goog.provide('jre.logging');

/** @define {string} */
jre.logging.logLevel =
goog.define('jre.logging.logLevel', goog.DEBUG ? 'ALL' : 'SEVERE');
jre.addSystemPropertyFromGoogDefine(
'jre.logging.logLevel', jre.logging.logLevel);

/** @define {string} */
jre.logging.simpleConsoleHandler =
goog.define('jre.logging.simpleConsoleHandler', 'ENABLED');
jre.addSystemPropertyFromGoogDefine(
'jre.logging.simpleConsoleHandler', jre.logging.simpleConsoleHandler);

// Provide a stub for field preserver to make things work without full
// compilation.
Expand Down
13 changes: 5 additions & 8 deletions jre/java/javaemul/internal/nativebootstrap/Util.impl.java.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,14 @@ class Util {
* if it is not defined.
*
* @param {string} name
* @param {?string=} opt_defaultValue
* @param {?string=} defaultValue
* @return {?string}
* @public
* @noinline
*/
static $getDefine(name, opt_defaultValue) {
// Default the optional param. Note that we are not using the common
// 'opt_value || default_value' pattern otherwise that would replace
// empty string with null value.
var defaultValue = opt_defaultValue == null ? null : opt_defaultValue;
var rv = goog.getObjectByName(name);
return rv == null ? defaultValue : String(rv);
// TODO(b/374872678): Remove the indirection through $getDefine.
static $getDefine(name, defaultValue = null) {
return jre.getSystemProperty(name, defaultValue);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
java.lang.RuntimeException: __the_message__!
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction$MyFunctionImpl.run(KotlinThrowsInJsFunction.kt:50)
at lambda(jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:199:31)
at lambda(jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:196:31)
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction.executesFunction(KotlinThrowsInJsFunction.kt:55)
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction$1.run(KotlinThrowsInJsFunction.kt:41)
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction.executesFunction(KotlinThrowsInJsFunction.kt:55)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
java.lang.RuntimeException: __the_message__!
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction$MyFunctionImpl.run(KotlinThrowsInJsFunction.kt:50)
at apply(<blaze-out>/jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:199:31)
at apply(<blaze-out>/jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:196:31)
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction.executesFunction(KotlinThrowsInJsFunction.kt:55)
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction$1.run(KotlinThrowsInJsFunction.kt:41)
at com.google.j2cl.junit.integration.stacktrace.data.KotlinThrowsInJsFunction.executesFunction(KotlinThrowsInJsFunction.kt:55)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
java.lang.RuntimeException: __the_message__!
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction$MyFunctionImpl.run(ThrowsInJsFunction.java:55)
at lambda(jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:199:31)
at lambda(jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:196:31)
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction.executesFunction(ThrowsInJsFunction.java:60)
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction$1.run(ThrowsInJsFunction.java:46)
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction.executesFunction(ThrowsInJsFunction.java:60)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
java.lang.RuntimeException: __the_message__!
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction$MyFunctionImpl.run(ThrowsInJsFunction.java:55)
at apply(<blaze-out>/jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:199:31)
at apply(<blaze-out>/jre/java/jre.js/javaemul/internal/nativebootstrap/Util.impl.java.js:196:31)
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction.executesFunction(ThrowsInJsFunction.java:60)
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction$1.run(ThrowsInJsFunction.java:46)
at com.google.j2cl.junit.integration.stacktrace.data.ThrowsInJsFunction.executesFunction(ThrowsInJsFunction.java:60)
Expand Down

0 comments on commit 7a3f585

Please sign in to comment.