[CLJS-670] Fix/enhance ^:export to work with properties that will be modified externally Created: 08/Nov/13 Updated: 11/Nov/13 Resolved: 11/Nov/13
Thin line between bug/enhancement here, but whatever...
Given this code:
Advanced compilation yields code where baz is inlined, and even the property itself disappears. Whatever a downstream user/app/lib does to foo.bar.baz, quux will always return 42. Here's that code:
Initially, I thought this was only because baz was a scalar, but the problem remains the same if an exported fn is returned by another fn; if the former is set to a different value downstream, the latter will continue returning the original that it closed over.
Based on this exchange with a GClosure compiler contributor, we need to use the @expose annotation on each exported property, and export the symbol for each intervening "namespace" segment in order to ensure that functions that refer to exported properties will return the same value that downstream users (might) mutate.
|Comment by Chas Emerick [ 08/Nov/13 2:50 PM ]|
Attached patch that changes the effect of ^:export as described above. With this patch, this input:
yields this under advanced compilation:
No exported properties are inlined, and external modifications to baz or a-fn affect functions that refer to them as expected.
|Comment by Chas Emerick [ 08/Nov/13 4:24 PM ]|
BTW, please consider that patch a draft. I'd like to work out some way to test this...
|Comment by David Nolen [ 09/Nov/13 7:38 PM ]|
Approach seems reasonable.
|Comment by Chas Emerick [ 09/Nov/13 8:52 PM ]|
It's subtly broken, discovered only after using it to build/test some real code. Working on it.
|Comment by Chas Emerick [ 11/Nov/13 10:57 AM ]|
There are various problems with the attached patch / approach, all rooted in GClosure's @expose:
1. More than preventing the munging/renaming/etc of a property, @expose disables a variety of advanced optimizations and analysis. When applied to functions, this will result in potentially a significant perf hit compared to what CLJS emits now.
Much of the above is reiterated elsewhere on the web, e.g. https://zooskdev.wordpress.com/2013/11/05/integrating-angularjs-with-a-large-google-closure-codebase/
Stepping back a bit, the objective of ^:export is solely to ensure that the affected var/property retains the name we give it, nothing else. GClosure provides an escape hatch for preventing renaming already — string-based property access — which, when combined with exporting of the "namespace"'s property, would address the problems with the existing ^:export impl.
The big hurdle is that the compiler will need to use string-based property access when touching any ^:export ed var/property. Lifting this information into the compiler environment so it can be looked up is easy; the delicate part will be changing any part of the compiler that emit var/property names to use a helper that distinguishes between locals and vars and uses string-based property access for the latter that are ^:export ed. This seems quite doable, but would be a more significant compiler change than simply twiddling goog.export* calls and such, so I thought I'd solicit feedback before moving forward.
|Comment by Chas Emerick [ 11/Nov/13 11:18 AM ]|
Per feedback from @dnolen in #clojure, ^:export is not intended to support mutable properties, and the changes necessary to add that support are undesirable.