Advanced optimization can break due to compiler hack being overwritten
awkay opened this issue · 7 comments
Using cljs 1.9.671 and om-beta1. Have verified in the compiled js (optimization none) that static methods are not getting @nocollapse, and are be elided by Closure.
Using cljsbuild 1.1.6 to build.
Initial investigation shows that the intern hack in next.cljc is not taking effect. I've checked my classpath to ensure there isn't an AOT compiler being used, and everything looks right.
@awkay this sounds like a regression in ClojureScript?
No, Antonio had put a hack into next.cljc that hacks the compiler to add nocollapse. The hack seems to not be working anymore, but I can't see where it is going wrong. It isn't getting installed anymore, and I'm not getting an AOT compiler on my classpath.
It seems to me that this is certainly something that could be fixed VIA the compiler proper, but I'm not sure if it would be purely for the Om Next users, since "static" additions to protocols is at the heart of it.
Location of hack: https://github.com/omcljs/om/blob/master/src/main/om/next.cljc#L313
OK, I tinkered some more with this. when I pare down the project to nothing but Om Next, then the correct @nocollapse annotations appear. So, my suspicion that the compiler hack is getting short-circuited are correct. Is there some way this can be moved into the compiler proper?
I tracked down the specific issue, and I was correct in my assessment: something was stomping on the hack. Our production compile had namespace refresh tools on the classpath that were triggered once on initial load which was causing the compiler, but for some reason not Om, to get refreshed in the runtime (possibly just out of order).
So, I still consider this a problem in Om, since a library should not hack the compiler...but I've at least found a solution to my specific instance of it going wrong.
I fixed this in Fulcro by changing defui to emit a try/catch of calls to the protocol methods via the class. This let's Closure understand that they are needed, and prevents them from being removed without having to hack into the cljs compiler.
The fix is this, which may apply cleanly as a patch to Om Next: