mihaip / react-closure-compiler

Tooling to teach Closure Compiler about React
Apache License 2.0
100 stars 14 forks source link

use CompilerAccessor instead of reflection #22

Closed rgpower closed 7 years ago

rgpower commented 7 years ago

I am hoping you might consider this pull request, which proposes to use a strategy similar to that employed in https://github.com/bolinfest/plovr/blob/master/src/com/google/javascript/jscomp/PlovrCompilerOptions.java

where you make class that exists in the same package as that protected method. In this case, we can expose the protected method via static call:

package com.google.javascript.jscomp;

public class CompilerAccessor {
  public static CompilerInput getSynthesizedExternsInputAtEnd(Compiler compiler) {
    return compiler.getSynthesizedExternsInputAtEnd();
  }
}

then your use of that would look like:

  private void addExterns() {
    CompilerInput externsInput = CompilerAccessor.getSynthesizedExternsInputAtEnd(compiler);
    addExternModule("React", "ReactModule", externsInput);
    addExternModule("ReactDOM", "ReactDOMModule", externsInput);
    addExternModule("ReactDOMServer", "ReactDOMServerModule", externsInput);
    compiler.reportCodeChange();
  }

This makes the usage of getSynthesizedExternsInput a little more declarative than reflection's imperative nature. Then, in the future, should the compiler change this method name, the error will be at compile time, instead of at runtime.

Then only downside I can think of in doing it this way, rather than reflection, would be if you are using this in an environment that requires sealed jar files. In that case you could simply move the compiler accessor class into the sealed jar I guess.

Also, update lib to include latest released closure-compiler from https://developers.google.com/closure/compiler/#how-do-i-start

this necessitated updating one of the unit tests.

lib/closure-compiler-20160911.jar -> lib/compiler-v20161024.jar

mihaip commented 7 years ago

Thanks!