System.getProperty doesn't emit goog.require

[niloc132]

Observed in jsinterop-base, where jsinterop.js contains the following:

/**
 * This file provides the @defines for jsinterop configuration options.
 * See InternalPreconditions.java for details.
 */

goog.provide('jsinterop');

// Note that disabling checking only disables it for production.

/** @define {string} */
goog.define('jsinterop.checks', 'DISABLED');

Then, InternalPreconditions.java uses this:

private static final boolean IS_TYPE_CHECKED = 
                getProperty("jsinterop.checks").equals("ENABLED");

Currently, System.getProperty is implemented with JsMethod for J2CL (and a magic method for gwt2):

  @JsMethod(name = "$getDefine", namespace = "nativebootstrap.Util")
  public static native String getProperty(String key);

The Util.$getDefine method only calls goog.getObjectByName with the parameter, not a compiletime string, and closure-compiler doesn't seem to be able to detect this anyway.

The result is a runtime NPE in both BUNDLE and ADVANCED compilation modes, unless jsinterop.js is manually added as an --entry_point, or a new define is added to the build directly (and in that case, --define isn't used by BUNDLE, by design).

Historically, it looks like J2CL used to have a pass which would examine System.getProperty calls and do something with them - perhaps the same idea could be used again to emit goog.require calls to correctly require that non-native.js files are loaded.

This might be worth generalizing, so that non-native.js files can be depended on in general? It would add an extra step to use a system property which isn't ideal, but does prevent the need for j2cl to recognize specific pieces of code to be transpiled.

This would also allow jre.js to keep working if classMetadata and checkedMode were moved - while there is a goog.provide('jre.checks') in that file, nothing actually ever depends on jre.checks, so could stop working if the file were split into one file per provide.

[gkdn]

I think the best way forward is to have goog.getDefine and have the tooling handle this but earlier they were hesitant on that.

[concavelenz]

What would "goog.getDefine" be?

[gkdn]

It would be an official version of J2CL's nativebootstrap.Util.$getDefine; a utility that returns the value of a property defined with @define.

See the current code for uncompiled mode here (minus the string coercion):

j2cl/jre/java/javaemul/internal/nativebootstrap/Util.impl.java.js

Line 39 in 1c04090

static $getDefine(name, opt_defaultValue) {

And the pass for compiled mode:
https://github.com/google/closure-compiler/blob/85d4241e6488ffe6e38496b3dc66c02a825f7ded/src/com/google/javascript/jscomp/J2clUtilGetDefineRewriterPass.java#L23

We are missing an automatic edge between the class that uses getDefine and the class that defines it.

[concavelenz]

This is unlikely to be a thing in Closure Library. Continuing to expose define values as globals is not something we want to do. global namespaces are slowly being turned down (nothing new should use goog.provide or goog.module.declareLegacyNamespace). Going forward, you should expect @define values to be module local and to access them you will need to import the module that defines them. Any solution you build around @define should keep that model in mind.

[gkdn]

The last time we discussed about @define couple of years back, the semantics around module @define wasn't clear (how it is set, how it is accessed etc.). Is there more clarification around it? Also what is the plan for existing stuff (DEBUG, COMPILED etc.)?

[jDramaix]

@concavelenz @gkdn any update?

[gkdn]

This doesn't need an update, could stay as a known issue.

[gkdn]

Since there is not much we can do here I'm closing the issue.
#76 tracks the documentation/example/macro.