Refactor CommonJS rewriting to be a normal pass #2691
Conversation
ChadKillingsworth
requested review from
lauraharker
and removed request for
lauraharker
ChadKillingsworth
force-pushed
the
common-js-pass
branch
from
ecb7105
to
549ebc1
Compare
| import com.google.common.collect.ImmutableList; | ||
| import com.google.javascript.rhino.Node; | ||
|
|
||
| /** Unit tests for {@link RewriteJsonToModule} */ |
There was a problem hiding this comment.
/** Unit tests for {@link Es6RewriteScriptsToModules} */
| @@ -615,6 +615,9 @@ abstract void updateGlobalVarReferences(Map<Var, ReferenceCollection> | |||
| */ | |||
| abstract ModuleLoader getModuleLoader(); | |||
|
|
|||
| /** Lookup the type of a module from its name. */ | |||
| abstract CompilerInput.ModuleType getModuleTypeByName(String moduleName); | |||
There was a problem hiding this comment.
I'm not following why we need this method. It looks like all the callers already have access to the CompilerInput in question, and that already contains the getJsModuleType() method.
There was a problem hiding this comment.
This actually stems from other work which has yet to be PR'd. It makes much more sense to use this directly off the input though, so I've switched it.
| } | ||
|
|
||
| /** Return whether or not the given script node represents an ES6 module file. */ | ||
| public static boolean isEs6ModuleRoot(Node scriptNode) { |
There was a problem hiding this comment.
Looks like now we have 3 definitions of this method. I'd probably get rid of all but one.
|
@blickly Changes made. Let me know when you are ready to import and I'll squash the commits. |
|
LGTM, let's squash and import. Thanks Chad! |
ChadKillingsworth
force-pushed
the
common-js-pass
branch
from
c84ebbe
to
435d743
Compare
|
@blickly Sqaushed |
Reworks CommonJS module rewriting as well as imported script rewriting into normal passes. Moves them into the default pass configuration.
Rewriting JSON files will be moved in a separate PR.