Aaaah

Ok

@dnolen which problems did you discover in the closure-compiler upgrade? The biggest one as far as shadow-cljs is concerned is the rename of JSModule (and variants) to JSChunk, after renaming references the code seems to work. I changed JSModule and JSModuleGraph in shadow-cljs. The type of the injectedLibraries field also changed but I don't think CLJS uses that

I get more issues in the closure-library upgrade, eg. goog.module.ModuleLoader is now a module and no longer exporting the legacy namespace. thus needs to be used via var thing = goog.require("goog.module.ModuleManager"), ie. using the return value of goog.require which none of the CLJS code currently does

@thheller we need to switch to builder pattern for SourceFile - JSChunk is the other thing - re: modules, I suppose we already assign some require patterns and we could do that in this case - but I guess it's annoying because it's a conditional thing

Hrm but is this necessary for ClojureScript? I thought we already addressed this problem - we transpile modules now during dev to lower them

it works fine if the goog.module has the goog.module.declareLegacyNamespace(), some of the newer ones dont have that

Ah right

So that has to be detected

well the next problem is that goog.require only has a return value when inside goog.module IIRC

But that’s not a problem right? You can only use the pattern if you’re loading a module that requires it

Detecting non legacy modules seems like a regex

not sure. I have a closure .js file in shadow-cljs that I couldn't get to work using the old style. had to rewrite it to also be a goog.module to import goog.module.ModuleLoader

Ok interesting - we shall see

I'd assume that CLJS has the same issue when trying to use those but this might have been because shadow-cljs doesn't use the default closure debug loader at all

Yes we use the debug loader

so that may have been the root cause. didn't test much further

So we just might not have this problem

It all seems familiar like I already saw all these cases

Ah but it was in closure lib itself

It was breaking the pattern - the ns assignment thing in closure itself

That what was fixed

So there’s probably still this edgecase for direct non legacy usage of modules

@thheller thanks for the notes will save some time

I briefly thought about just adding (str code "\ngoog.module.declareLegacyNamespaces();") for goog.module files that don't have it when feeding the transpiler

might be a hack but might work

we already pre-load all the Closure nses and collect our own info - so I think parsing and emitting differently is not hard