closure-compiler: Cannot use -D for @define constant within a module

Repro case:

// a.js
/** @define {string} */ var FOO = 'default foo';
export function test() { console.log(FOO); }

// b.js
import * as a from './a';
a.test();

java -jar compiler.jar --js *.js \
    --language_in=ECMASCRIPT6_STRICT \
    --language_out=ECMASCRIPT5_STRICT \
    -D "FOO='hi'";

WARNING - unknown @define variable FOO

0 error(s), 1 warning(s)
'use strict';var module$a={},FOO$$module$a="default foo";function test$$module$a(){console.log(FOO$$module$a)}module$a.test=test$$module$a;module$a.test();

Things work if you use the mangled name:

java -jar compiler.jar --js *.js \
    --language_in=ECMASCRIPT6_STRICT \
    --language_out=ECMASCRIPT5_STRICT \
    -D "FOO\$\$module\$a='hi'";

'use strict';var module$a={},FOO$$module$a="hi";function test$$module$a(){console.log(FOO$$module$a)}module$a.test=test$$module$a;module$a.test();

About this issue

Original URL
State: open
Created 8 years ago
Reactions: 1
Comments: 27 (18 by maintainers)

Commits related to this issue

Don't rename @define's in ProcessEs6Modules Treat @define's as truly global definitions and don't mangle the names. Fixes #1601. — committed to Dominator008/closure-compiler by Dominator008 8 years ago
Don't rename @define's in ProcessEs6Modules Treat @define's as truly global definitions and don't mangle the names. Fixes #1601. — committed to Dominator008/closure-compiler by Dominator008 8 years ago
Make separate entry point for compo build This was a bit of a pain to get working. The closure compiler appears to include modules even if they are not dependencies of the entry point. It requires so... — committed to depp/hdd-liberator by depp 3 years ago
chore: turns out we need to use goog.define https://github.com/google/closure-compiler/issues/1601#issuecomment-483452226 — committed to hartmannr76/swg-js by hartmannr76 a year ago

Most upvoted comments

My vote would go to “treat all defines as truly global and do not mangle their names”. If I supply a -D argument to a build process, I do not care in which file the define is located.

ray007 on Aug 23, 2018

Still want this to be resolved…

One more idea how to deal with this; Let -Dxyz=123 define a mapping from xyz to 123. When we see a @define named xyz in any module we replace the value. That is it. That means that if there are multiple defines named the same thing they will share the value. In practice I don’t think this is an issue. This also means that files need to import and export the @defined bindings but it leads to less confusion and works better with tools such as eslint and vscode anyway.

arv on Apr 15, 2019

We’ve made some improvements, but the story is pretty different depending on whether or not you’re using Closure Library. TL;DR: goog.define works, but raw @define doesn’t really.

With Closure Library

goog.define now works correctly in modules by returning its value to be stored in a module-local variable, with the caveat that it does still export global values for the time being. In short, you can write in a module

/** @define {number} */
const FOO = goog.define('some.ns.FOO', 1);

and then set it from the command line with --define some.ns.FOO=42. The string identifier must be a valid JS qualified name, but aside from linking the command-line flag to the specific definition, it has no other use (once we stop exporting it as global—which will happen in the next few months, once we’ve cleaned up everything that would break).

This has sort of worked in the past, but in an unsupported way: in compiled code it defines a module-local with no global, while in uncompiled code it defined a global. The main breakages that need to be fixed before we can stop exporting the global are based on folks exploiting this inconsistency to change @defined values in uncompiled tests.

Without Closure Library

Without goog.define, the story is unfortunately a bit more complicated. You can’t just write /** @define {number} */ const FOO = 1; and -DFOO because module rewriting happens before defines are processed, so you’d need to write a munged name on the command line. This munging of course is an implementation detail and depends on the kind of module (goog, ES, CJS) as well as whether it’s exported or not. I don’t know of a good way to determine the munged name (short of changing the compiler to print allDefines.keySet() at the end of ProcessDefines#overrideDefines), nor do I think it’s a particularly good idea. It would be possible to rearrange it so that it’s keyed on the unmunged name, but I worry that that would break folks that already are (unwisely or not) depending on this munging.

Independent of this, it’s still an error to define the same symbol twice. We could revisit that, but I’m not convinced that would be a good idea.

shicks on Apr 15, 2019

Here is what I had in mind: We need to add support for modules, a define for a module would look something like this:

-D “path/to/module.js:NAME=1”

for the define to be visible outside the module, it would need to be exported separately. Defines in modules should only be allowed on const declarations:

/** @define {boolean} */ const FOO = false;

goog.define also needs to be modified for ES6 modules as they don’t know their own name so it is difficult to look them up at runtime.

Alternately, we require all @define values to be globally unique but that would eventually going to cause problems.

concavelenz on Nov 22, 2016

I thought of a better way to handle this: during module rewriting, rewrite the define flags in a similar way that rewrite type nodes.

ChadKillingsworth on Nov 19, 2016