--replace_strings does not understand goog.module names

[domnit]

Given this code:

// err.js
goog.module('example.Err');
class Err extends Error {}
exports = Err;

// thrower.js
goog.module('example.thrower');
const Err = goog.require('example.Err');
function throwErr() {
  throw new Err('please replace me');
}

String replacement does not occur when compiling with --replace_strings='example.Err(?)'.

It does occur when compiling with --replace_strings='Err(?)'.

Just using the short name is not a great solution, since short names can conflict, and module names can be shortened differently, across different modules in the compilation unit.

The same issue exists with goog.scope, but then at least you can simply avoid scoping the error type. With goog.module, the namespace must be removed, so there is no way to specify that a string replacement should be applied to a certain error type.

[brad4d]

Name comparison is currently done on the original qualified name of the alias.
https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/ReplaceStrings.java#L206

In the example above that means it's matching against the name given where the module is required.

const Err = goog.require('example.Err');

If different files import the same module with different names for their constants, you'll have to include all of them in your --replace_strings= flags. Also if you created an Err that was totally unrelated, replacement could still happen on it.

Currently you could work around this but still use goog.module by using goog.declareLegacyNamespace() and then continuing to access the error classes by full paths instead of using local aliases.

[domnit]

Thanks Bradford.

I understand that string replacement works on the name given where the module is required, so sort-of working-as-intended, but I would argue that the behavior is not a good fit for the way goog.module and string replacement are used.

Specifically, I would expect to be able to register a given error type once and have string replacement occur for all uses of that type. I would also expect to not inadvertently cause string replacement in an unrelated call because of a short name collision across unrelated files.

I'd be happy to give more context on my use case (gws) over Google internal channels, if that would be helpful.

[brad4d]

@domnit
To be clear, I wasn't trying to say this is WAI.
I was just adding some more explanation of what is happening and suggesting a workaround.

I agree that the way this should work is you should be able to say "replace strings passed as the 3rd argument to calls to method X on class Y imported from module M".