|
|
Patch Set 1 : Update patch #
Total comments: 1
MessagesTotal messages: 14
Looong comment, partially devised from thinking about this for a while... http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... File features/src/main/javascript/features/rpc/rpc.js (right): http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... features/src/main/javascript/features/rpc/rpc.js:721: gadgets.rpc.config = function(config) { As I've been thinking about this some more, I'm starting to think that our best solution is going to be an improved export mechanism. It helps me to state quite simply what the goals are. 1. Enable property obfuscation. 2. Enable exports-as-imports. 3. Enable export of only "requested" symbol features to uncompiled users. As written here, I'm not sure how we'd export gadgets.rpc, either using the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any other, since we're hiding the gadgets.rpc decl within an anonymous closure. Interestingly, in the runtime-compiled case the existing, unmodified mechanism achieves property compression in the ideal fashion: users of gadgets.rpc get properly translated eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will get obfuscated to something like: g.P = (function() { // "gadgets.rpc = ..." stuff return { Q: function() { /* call(...) method */ }, R: function() { /* setupReceiver(...) method */ }, }; })(); dynamic-height, being compiled in the same unit, will have: gadgets.rpc.call(...) ...changed to: g.P.Q(...) ...which works. The main challenge is when someone wants access to g.P.Q by calling gadgets.rpc.call. For this, we need a smarter export method. I wonder if something like this would work: jsExport(obj, namespace, propMap); For gadgets.rpc, we'd generate: jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: "setupReceiver", ... }); When compiled, this turns into: yZ(g.P, { Q: "call", R: "setupReceiver" }); The jsExport method would then: 1) if propMap && typeof obj === "object", A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the given symbol on window. B) export properties directly by name, as singletons. Iterate through propMap, exporting properties by name, eg. window["gadgets"]["rpc"]["call"] = g.P["Q"]; 2) else if propMap && typeof obj === "function", assume to be a Closured-style object constructor. A) do same as #1, only export window["foo"]["bar"] as a function that in turn exports the properties it has been given, by name. This whole edifice would allow us to: 1. Use property compression optimally. 2. Export only "some" symbols externally. 3. Export singletons, Closured, and prototyped objects the same way: <api> <exports type="js">foo.bar.method1</exports> <exports type="js">foo.bar.method2</exports> ... </api> wdyt?
Sign in to reply to this message.
On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: > Looong comment, partially devised from thinking about this for a > while... > Thanks for starting this thoughtful discussion. > > > http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... > File features/src/main/javascript/features/rpc/rpc.js (right): > > > http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... > features/src/main/javascript/features/rpc/rpc.js:721: gadgets.rpc.config > = function(config) { > As I've been thinking about this some more, I'm starting to think that > our best solution is going to be an improved export mechanism. > > It helps me to state quite simply what the goals are. > 1. Enable property obfuscation. > 2. Enable exports-as-imports. > Please elaborate this one. > 3. Enable export of only "requested" symbol features to uncompiled > users. > As written here, I'm not sure how we'd export gadgets.rpc, either using > the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any > other, since we're hiding the gadgets.rpc decl within an anonymous > closure. > > Interestingly, in the runtime-compiled case the existing, unmodified > mechanism achieves property compression in the ideal fashion: users of > gadgets.rpc get properly translated > > eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will get > obfuscated to something like: > g.P = (function() { // "gadgets.rpc = ..." > stuff > return { > Q: function() { /* call(...) method */ }, > R: function() { /* setupReceiver(...) method */ }, > }; > })(); > > dynamic-height, being compiled in the same unit, will have: > gadgets.rpc.call(...) > ...changed to: > g.P.Q(...) > > ...which works. > > Correct this works. But, currently, anything (say a new function X) return {}'ed from gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if 1) not referenced from global/window, 2) not part of "requested" symbols, 3) never used by client code. However, it should be removed for EXPLICIT_RUN_TIME. > The main challenge is when someone wants access to g.P.Q by calling > gadgets.rpc.call. For this, we need a smarter export method. I wonder if > something like this would work: > I don't follow here, the above example is someone calling gadgets.rpc.call() and it translates fine to g.P.Q, no? jsExport(obj, namespace, propMap); > > For gadgets.rpc, we'd generate: > > jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: > "setupReceiver", ... }); > > When compiled, this turns into: > yZ(g.P, { Q: "call", R: "setupReceiver" }); > > The jsExport method would then: > 1) if propMap && typeof obj === "object", > A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the > given symbol on window. > B) export properties directly by name, as singletons. Iterate through > propMap, exporting properties by name, eg. > window["gadgets"]["rpc"]["call"] = g.P["Q"]; > Who's generating these jsExport(...) statements ? The JS serving mechanism based on current <exports> or JS developer ? > 2) else if propMap && typeof obj === "function", assume to be a > Closured-style object constructor. > A) do same as #1, only export window["foo"]["bar"] as a function that > in turn exports the properties it has been given, by name. > If I understand this correctly, you want to allow 2 styles to have symbols exported. Does one offer a benefit that the other doesn't? I personally would go after 1 style, that works. Refactoring all to either style is easy. And I personally think the object-style declaration is more obvious -- symbols don't get mapped by some JS execution/helper, less indirections (ie: no return statements). Thoughts? How about internal objects like rpc.[a|c|t ...], do those need to be quoted still? > This whole edifice would allow us to: > 1. Use property compression optimally. > Yes. > 2. Export only "some" symbols externally. > Yes. Currently, "some" is determined by symbols exported in explicitly-listed features. > 3. Export singletons, Closured, and prototyped objects the same way: > <api> > <exports type="js">foo.bar.method1</exports> > <exports type="js">foo.bar.method2</exports> > ... > How do these export prototyped objects? > </api> > > wdyt? > > > http://codereview.appspot.com/4171051/ > Im putting a prototype covering these different cases.
Sign in to reply to this message.
Michael - Great to get this going. Comments inline. On Mon, Feb 14, 2011 at 6:21 PM, Michael Hermanto <mhermanto@gmail.com>wrote: > > > On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: > >> Looong comment, partially devised from thinking about this for a >> while... >> > > Thanks for starting this thoughtful discussion. > > >> >> >> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >> File features/src/main/javascript/features/rpc/rpc.js (right): >> >> >> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >> features/src/main/javascript/features/rpc/rpc.js:721: gadgets.rpc.config >> = function(config) { >> As I've been thinking about this some more, I'm starting to think that >> our best solution is going to be an improved export mechanism. >> >> It helps me to state quite simply what the goals are. >> 1. Enable property obfuscation. >> 2. Enable exports-as-imports. >> > > Please elaborate this one. > Aka "exports-as-externs," the ability to generate an "externs" symbol file (for compilation) from a given JS bundle without doing so manually. For the vast majority of cases this won't even be needed though, since JS using these libs will either be included in the bundle (so the props will compile appropriately) or be written in some other way, such as inlined JS in HTML using gadgets.rpc.call(..) > > >> 3. Enable export of only "requested" symbol features to uncompiled >> users. > > > >> As written here, I'm not sure how we'd export gadgets.rpc, either using >> the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any >> other, since we're hiding the gadgets.rpc decl within an anonymous >> closure. >> >> Interestingly, in the runtime-compiled case the existing, unmodified >> mechanism achieves property compression in the ideal fashion: users of >> gadgets.rpc get properly translated >> >> eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will get >> obfuscated to something like: >> g.P = (function() { // "gadgets.rpc = ..." >> stuff >> return { >> Q: function() { /* call(...) method */ }, >> R: function() { /* setupReceiver(...) method */ }, >> }; >> })(); >> >> dynamic-height, being compiled in the same unit, will have: >> gadgets.rpc.call(...) >> ...changed to: >> g.P.Q(...) >> >> ...which works. >> >> > Correct this works. > > But, currently, anything (say a new function X) return {}'ed from > gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if > 1) not referenced from global/window, 2) not part of "requested" symbols, 3) > never used by client code. However, it should be removed for > EXPLICIT_RUN_TIME. > That's true for anything returned like: return { 'symbol': foo, 'another': bar }; I'm actually now agreeing w/ you that property compaction is a very good idea, so we should not quote these. As discussed above, we can turn on property compaction and all JS in the compiled bundle will work appropriately. @see next comment for more. > > >> The main challenge is when someone wants access to g.P.Q by calling >> gadgets.rpc.call. For this, we need a smarter export method. I wonder if >> something like this would work: >> > > I don't follow here, the above example is someone calling > gadgets.rpc.call() and it translates fine to g.P.Q, no? > Yes. I should have stated more clearly -- the main challenge is when an "external" caller (not in the JS bundle) wants to reference g.P.Q by its exported name, gadgets.rpc.call. > > jsExport(obj, namespace, propMap); >> >> For gadgets.rpc, we'd generate: >> >> jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: >> "setupReceiver", ... }); >> >> When compiled, this turns into: >> yZ(g.P, { Q: "call", R: "setupReceiver" }); >> >> The jsExport method would then: >> 1) if propMap && typeof obj === "object", >> A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the >> given symbol on window. >> B) export properties directly by name, as singletons. Iterate through >> propMap, exporting properties by name, eg. >> window["gadgets"]["rpc"]["call"] = g.P["Q"]; >> > > Who's generating these jsExport(...) statements ? The JS serving mechanism > based on current <exports> or JS developer ? > JS serving mechanism. The difference is that instead of exporting directly on window, it delegates to a "smart" JS function to do so. So if your <exports> are: gadgets.flash.embedFlash gadgets.rpc.call gadgets.rpc.setupReceiver ...your generated exports are currently something like this: window["gadgets"]=gadgets; window["gadgets"]["rpc"]=gadgets.rpc; window["gadgets"]["rpc"]["call"]=gadgets.rpc.call; etc etc With the help of a smart export function, we can make this both richer (supporting exported closured-Objects) and more compact. The minimal would be: jsExport("gadgets.flash", [ gadgets, gadgets.flash, gadgets.flash ], { embedFlash: "embedFlash" }); jsExport("gadgets.rpc", [ gadgets, gadgets.rpc ], { call: "call", setupReceiver: "setupReceiver" }); The implementation for jsExport: function jsExport(namespace, components, opt_props) { var nsParts = namespace.split("."); var base = window; var prevBase = null; var part = null; for (var i=0; part=nsParts.shift();) { // export "namespaces" base[part]=components[i++]; prevBase = base; base=base[part]; } var props = opt_props || {}; var exportProps = function(root) { for (var prop in props) { if (props.hasOwnProperty(prop)) { root[props[prop]] = root[prop]; } } }; if (typeof base === "object") { // "singleton" object ie. gadgets.rpc w/ props "call", "setupReceiver" etc. // just export the symbols immediately exportProps(root); } else if (typeof base === "function") { // "closured" object such as shindig.uri with methods "getQuery", "setFragment" etc. // create an wrapper to this function that exports any resulting properties it returns in an Object var exportedFn = function() { var result = base.apply(null, arguments); if (typeof result === "object") { exportProps(result); } return result; }; prevBase[part] = exportedFn; } } > > >> 2) else if propMap && typeof obj === "function", assume to be a >> Closured-style object constructor. >> A) do same as #1, only export window["foo"]["bar"] as a function that >> in turn exports the properties it has been given, by name. >> > > If I understand this correctly, you want to allow 2 styles to have symbols > exported. Does one offer a benefit that the other doesn't? I personally > would go after 1 style, that works. Refactoring all to either style is easy. > And I personally think the object-style declaration is more obvious -- > symbols don't get mapped by some JS execution/helper, less indirections (ie: > no return statements). Thoughts? > AFAIK there will always be >1 way to write exported JS since you can have singletons and you can have constructed "Objects" with methods on them. Exporting singletons is pretty easy in any case. Exporting methods is harder when you use property compression. That's true whether you use Closured or prototyped objects, since methods on each are properties. One advantage of the above mechanism is that it works equally well for both styles of object. > > How about internal objects like rpc.[a|c|t ...], do those need to be quoted > still? > With an export method,* nothing* needs to be quoted, now that I think of it, unless that "thing" is an API that's used by non-compiled contexts. So: 1) internal property keys need not be quoted, since they're only referenced internally and thus obfuscated consistently 2) parameter-bag property keys *do* need to be quoted. Toy example: suppose we had library Point that has method setCoords(params) which takes { x: foo, y: foo }. 'x' and 'y' need to be quoted since external callers will pass in param bags with these as keys. 3) rpc.foo is an interesting case. Technically they do not need to be quoted, since in any situation both sides of gadgets.rpc communication should be using the same bundle and thus consistent obfuscations. However, we should probably quote them anyway for consistent debugging, and because bundles might be asymmetric yet complementary (one side has "rpc" while the other has "rpc:dynamic-height" leading to different obfuscations). > > >> This whole edifice would allow us to: >> 1. Use property compression optimally. >> > > Yes. > > >> 2. Export only "some" symbols externally. >> > > Yes. Currently, "some" is determined by symbols exported in > explicitly-listed features. > > >> 3. Export singletons, Closured, and prototyped objects the same way: >> <api> >> <exports type="js">foo.bar.method1</exports> >> <exports type="js">foo.bar.method2</exports> >> ... >> > How do these export prototyped objects? > @see above for current thought on syntax. Everything up to the last "." becomes the "root" namespace; all the properties for the same foo.bar prefix become the propMap. wdyt? --j > > >> </api> >> >> wdyt? >> >> >> http://codereview.appspot.com/4171051/ >> > > Im putting a prototype covering these different cases. >
Sign in to reply to this message.
On Tue, Feb 15, 2011 at 3:20 PM, John Hjelmstad <fargo@google.com> wrote: > Michael - > > Great to get this going. Comments inline. > > On Mon, Feb 14, 2011 at 6:21 PM, Michael Hermanto <mhermanto@gmail.com>wrote: > >> >> >> On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: >> >>> Looong comment, partially devised from thinking about this for a >>> while... >>> >> >> Thanks for starting this thoughtful discussion. >> >> >>> >>> >>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>> File features/src/main/javascript/features/rpc/rpc.js (right): >>> >>> >>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>> features/src/main/javascript/features/rpc/rpc.js:721: gadgets.rpc.config >>> = function(config) { >>> As I've been thinking about this some more, I'm starting to think that >>> our best solution is going to be an improved export mechanism. >>> >>> It helps me to state quite simply what the goals are. >>> 1. Enable property obfuscation. >>> 2. Enable exports-as-imports. >>> >> >> Please elaborate this one. >> > > Aka "exports-as-externs," the ability to generate an "externs" symbol file > (for compilation) from a given JS bundle without doing so manually. For the > vast majority of cases this won't even be needed though, since JS using > these libs will either be included in the bundle (so the props will compile > appropriately) or be written in some other way, such as inlined JS in HTML > using gadgets.rpc.call(..) > Is this merely just dumping all the content of <exports type=js> directives to a flat file ? I agree that we should only specify this in one place (feature.xml for features makes sense), but it'll also introduce another "way" to specify externs (something that people need to learn again). > > >> >> >>> 3. Enable export of only "requested" symbol features to uncompiled >>> users. >> >> >> >>> As written here, I'm not sure how we'd export gadgets.rpc, either using >>> the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any >>> other, since we're hiding the gadgets.rpc decl within an anonymous >>> closure. >>> >>> Interestingly, in the runtime-compiled case the existing, unmodified >>> mechanism achieves property compression in the ideal fashion: users of >>> gadgets.rpc get properly translated >>> >>> eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will get >>> obfuscated to something like: >>> g.P = (function() { // "gadgets.rpc = ..." >>> stuff >>> return { >>> Q: function() { /* call(...) method */ }, >>> R: function() { /* setupReceiver(...) method */ }, >>> }; >>> })(); >>> >>> dynamic-height, being compiled in the same unit, will have: >>> gadgets.rpc.call(...) >>> ...changed to: >>> g.P.Q(...) >>> >>> ...which works. >>> >>> >> Correct this works. >> >> But, currently, anything (say a new function X) return {}'ed from >> gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if >> 1) not referenced from global/window, 2) not part of "requested" symbols, 3) >> never used by client code. However, it should be removed for >> EXPLICIT_RUN_TIME. >> > > That's true for anything returned like: > return { > 'symbol': foo, > 'another': bar > }; > > I'm actually now agreeing w/ you that property compaction is a very good > idea, so we should not quote these. As discussed above, we can turn on > property compaction and all JS in the compiled bundle will work > appropriately. @see next comment for more. > > >> >> >>> The main challenge is when someone wants access to g.P.Q by calling >>> gadgets.rpc.call. For this, we need a smarter export method. I wonder if >>> something like this would work: >>> >> >> I don't follow here, the above example is someone calling >> gadgets.rpc.call() and it translates fine to g.P.Q, no? >> > > Yes. I should have stated more clearly -- the main challenge is when an > "external" caller (not in the JS bundle) wants to reference g.P.Q by its > exported name, gadgets.rpc.call. > > >> >> jsExport(obj, namespace, propMap); >>> >>> For gadgets.rpc, we'd generate: >>> >>> jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: >>> "setupReceiver", ... }); >>> >>> When compiled, this turns into: >>> yZ(g.P, { Q: "call", R: "setupReceiver" }); >>> >>> The jsExport method would then: >>> 1) if propMap && typeof obj === "object", >>> A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the >>> given symbol on window. >>> B) export properties directly by name, as singletons. Iterate through >>> propMap, exporting properties by name, eg. >>> window["gadgets"]["rpc"]["call"] = g.P["Q"]; >>> >> >> Who's generating these jsExport(...) statements ? The JS serving mechanism >> based on current <exports> or JS developer ? >> > > JS serving mechanism. The difference is that instead of exporting directly > on window, it delegates to a "smart" JS function to do so. > > So if your <exports> are: > gadgets.flash.embedFlash > gadgets.rpc.call > gadgets.rpc.setupReceiver > > ...your generated exports are currently something like this: > window["gadgets"]=gadgets; > window["gadgets"]["rpc"]=gadgets.rpc; > window["gadgets"]["rpc"]["call"]=gadgets.rpc.call; > etc etc > > With the help of a smart export function, we can make this both richer > (supporting exported closured-Objects) and more compact. The minimal would > be: > jsExport("gadgets.flash", [ gadgets, gadgets.flash, gadgets.flash ], { > embedFlash: "embedFlash" }); > jsExport("gadgets.rpc", [ gadgets, gadgets.rpc ], { call: "call", > setupReceiver: "setupReceiver" }); > > The implementation for jsExport: > > function jsExport(namespace, components, opt_props) { > var nsParts = namespace.split("."); > var base = window; > var prevBase = null; > var part = null; > for (var i=0; part=nsParts.shift();) { > // export "namespaces" > base[part]=components[i++]; > prevBase = base; > base=base[part]; > } > var props = opt_props || {}; > var exportProps = function(root) { > for (var prop in props) { if (props.hasOwnProperty(prop)) { > root[props[prop]] = root[prop]; > } } > }; > if (typeof base === "object") { > // "singleton" object ie. gadgets.rpc w/ props "call", "setupReceiver" > etc. > // just export the symbols immediately > exportProps(root); > } else if (typeof base === "function") { > // "closured" object such as shindig.uri with methods "getQuery", > "setFragment" etc. > // create an wrapper to this function that exports any resulting > properties it returns in an Object > var exportedFn = function() { > var result = base.apply(null, arguments); > if (typeof result === "object") { > exportProps(result); > } > return result; > }; > prevBase[part] = exportedFn; > } > } > > Thanks for making this easy. I'm trying this out now in AbstractJsCompiler. > >> >>> 2) else if propMap && typeof obj === "function", assume to be a >>> Closured-style object constructor. >>> A) do same as #1, only export window["foo"]["bar"] as a function that >>> in turn exports the properties it has been given, by name. >>> >> >> If I understand this correctly, you want to allow 2 styles to have symbols >> exported. Does one offer a benefit that the other doesn't? I personally >> would go after 1 style, that works. Refactoring all to either style is easy. >> And I personally think the object-style declaration is more obvious -- >> symbols don't get mapped by some JS execution/helper, less indirections (ie: >> no return statements). Thoughts? >> > > AFAIK there will always be >1 way to write exported JS since you can have > singletons and you can have constructed "Objects" with methods on them. > Exporting singletons is pretty easy in any case. Exporting methods is harder > when you use property compression. > > That's true whether you use Closured or prototyped objects, since methods > on each are properties. One advantage of the above mechanism is that it > works equally well for both styles of object. > > >> >> How about internal objects like rpc.[a|c|t ...], do those need to be >> quoted still? >> > > With an export method,* nothing* needs to be quoted, now that I think of > it, unless that "thing" is an API that's used by non-compiled contexts. > > So: > 1) internal property keys need not be quoted, since they're only referenced > internally and thus obfuscated consistently > Yes, also consistently across features (well, because the compiler knows all the features at once). > 2) parameter-bag property keys *do* need to be quoted. Toy example: > suppose we had library Point that has method setCoords(params) which takes { > x: foo, y: foo }. 'x' and 'y' need to be quoted since external callers will > pass in param bags with these as keys. > Yes. > 3) rpc.foo is an interesting case. Technically they do not need to be > quoted, since in any situation both sides of gadgets.rpc communication > should be using the same bundle and thus consistent obfuscations. However, > we should probably quote them anyway for consistent debugging, and because > bundles might be asymmetric yet complementary (one side has "rpc" while the > other has "rpc:dynamic-height" leading to different obfuscations). > Yes, we've talked about this, rpc.[t|c|...] are used publicly by client (ie: rpc.f to find source caller). I don't think you intend clients to use this in the first place, because otherwise, I'd come up with more meaningful names. > >> >> >>> This whole edifice would allow us to: >>> 1. Use property compression optimally. >>> >> >> Yes. >> >> >>> 2. Export only "some" symbols externally. >>> >> >> Yes. Currently, "some" is determined by symbols exported in >> explicitly-listed features. >> >> >>> 3. Export singletons, Closured, and prototyped objects the same way: >>> <api> >>> <exports type="js">foo.bar.method1</exports> >>> <exports type="js">foo.bar.method2</exports> >>> ... >>> >> How do these export prototyped objects? >> > > @see above for current thought on syntax. Everything up to the last "." > becomes the "root" namespace; all the properties for the same foo.bar prefix > become the propMap. > > wdyt? > > One thought ... can we assume a feature can only export namespace (ie: everything before the last property) that is common across all, as long as not on window. This assumption appears to make the code simpler. ie: A feature cannot do -- <exports type=js>gadgets.util.makeClosure</exports> <exports type=js>gadgets.rpc.call</exports> // unless this translate to 2 jsExport statements, each for gadgets.util and gadgets.rpc. // I can't think of a non-global feature that does this. But a feature (like globals) can do -- <exports type=js>gadgets</exports> <exports type=js>shindig</exports> // translate to jsExport(null/window, [], { gadgets : "gadgets", shindig : "shindig }); --j > > >> >> >>> </api> >>> >>> wdyt? >>> >>> >>> http://codereview.appspot.com/4171051/ >>> >> >> Im putting a prototype covering these different cases. >> > >
Sign in to reply to this message.
On Wed, Feb 16, 2011 at 11:32 AM, Michael Hermanto <mhermanto@gmail.com>wrote: > > > On Tue, Feb 15, 2011 at 3:20 PM, John Hjelmstad <fargo@google.com> wrote: > >> Michael - >> >> Great to get this going. Comments inline. >> >> On Mon, Feb 14, 2011 at 6:21 PM, Michael Hermanto <mhermanto@gmail.com>wrote: >> >>> >>> >>> On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: >>> >>>> Looong comment, partially devised from thinking about this for a >>>> while... >>>> >>> >>> Thanks for starting this thoughtful discussion. >>> >>> >>>> >>>> >>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>> File features/src/main/javascript/features/rpc/rpc.js (right): >>>> >>>> >>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>> features/src/main/javascript/features/rpc/rpc.js:721: gadgets.rpc.config >>>> = function(config) { >>>> As I've been thinking about this some more, I'm starting to think that >>>> our best solution is going to be an improved export mechanism. >>>> >>>> It helps me to state quite simply what the goals are. >>>> 1. Enable property obfuscation. >>>> 2. Enable exports-as-imports. >>>> >>> >>> Please elaborate this one. >>> >> >> Aka "exports-as-externs," the ability to generate an "externs" symbol file >> (for compilation) from a given JS bundle without doing so manually. For the >> vast majority of cases this won't even be needed though, since JS using >> these libs will either be included in the bundle (so the props will compile >> appropriately) or be written in some other way, such as inlined JS in HTML >> using gadgets.rpc.call(..) >> > > Is this merely just dumping all the content of <exports type=js> directives > to a flat file ? I agree that we should only specify this in one place > (feature.xml for features makes sense), but it'll also introduce another > "way" to specify externs (something that people need to learn again). > Yes and no, to some extent. This is really only useful for external callers, such as users of this JS within Google who also use Closure Compiler. As you note, feature.xml is where exports will be. What I'd like is to avoid these users manually writing an externs.js file, and instead referring to one that's automatically-generated from the feature itself. > >> >>> >>> >>>> 3. Enable export of only "requested" symbol features to uncompiled >>>> users. >>> >>> >>> >>>> As written here, I'm not sure how we'd export gadgets.rpc, either using >>>> the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any >>>> other, since we're hiding the gadgets.rpc decl within an anonymous >>>> closure. >>>> >>>> Interestingly, in the runtime-compiled case the existing, unmodified >>>> mechanism achieves property compression in the ideal fashion: users of >>>> gadgets.rpc get properly translated >>>> >>>> eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will get >>>> obfuscated to something like: >>>> g.P = (function() { // "gadgets.rpc = ..." >>>> stuff >>>> return { >>>> Q: function() { /* call(...) method */ }, >>>> R: function() { /* setupReceiver(...) method */ }, >>>> }; >>>> })(); >>>> >>>> dynamic-height, being compiled in the same unit, will have: >>>> gadgets.rpc.call(...) >>>> ...changed to: >>>> g.P.Q(...) >>>> >>>> ...which works. >>>> >>>> >>> Correct this works. >>> >>> But, currently, anything (say a new function X) return {}'ed from >>> gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if >>> 1) not referenced from global/window, 2) not part of "requested" symbols, 3) >>> never used by client code. However, it should be removed for >>> EXPLICIT_RUN_TIME. >>> >> >> That's true for anything returned like: >> return { >> 'symbol': foo, >> 'another': bar >> }; >> >> I'm actually now agreeing w/ you that property compaction is a very good >> idea, so we should not quote these. As discussed above, we can turn on >> property compaction and all JS in the compiled bundle will work >> appropriately. @see next comment for more. >> >> >>> >>> >>>> The main challenge is when someone wants access to g.P.Q by calling >>>> gadgets.rpc.call. For this, we need a smarter export method. I wonder if >>>> something like this would work: >>>> >>> >>> I don't follow here, the above example is someone calling >>> gadgets.rpc.call() and it translates fine to g.P.Q, no? >>> >> >> Yes. I should have stated more clearly -- the main challenge is when an >> "external" caller (not in the JS bundle) wants to reference g.P.Q by its >> exported name, gadgets.rpc.call. >> >> >>> >>> jsExport(obj, namespace, propMap); >>>> >>>> For gadgets.rpc, we'd generate: >>>> >>>> jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: >>>> "setupReceiver", ... }); >>>> >>>> When compiled, this turns into: >>>> yZ(g.P, { Q: "call", R: "setupReceiver" }); >>>> >>>> The jsExport method would then: >>>> 1) if propMap && typeof obj === "object", >>>> A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the >>>> given symbol on window. >>>> B) export properties directly by name, as singletons. Iterate through >>>> propMap, exporting properties by name, eg. >>>> window["gadgets"]["rpc"]["call"] = g.P["Q"]; >>>> >>> >>> Who's generating these jsExport(...) statements ? The JS serving >>> mechanism based on current <exports> or JS developer ? >>> >> >> JS serving mechanism. The difference is that instead of exporting directly >> on window, it delegates to a "smart" JS function to do so. >> >> So if your <exports> are: >> gadgets.flash.embedFlash >> gadgets.rpc.call >> gadgets.rpc.setupReceiver >> >> ...your generated exports are currently something like this: >> window["gadgets"]=gadgets; >> window["gadgets"]["rpc"]=gadgets.rpc; >> window["gadgets"]["rpc"]["call"]=gadgets.rpc.call; >> etc etc >> >> With the help of a smart export function, we can make this both richer >> (supporting exported closured-Objects) and more compact. The minimal would >> be: >> jsExport("gadgets.flash", [ gadgets, gadgets.flash, gadgets.flash ], { >> embedFlash: "embedFlash" }); >> jsExport("gadgets.rpc", [ gadgets, gadgets.rpc ], { call: "call", >> setupReceiver: "setupReceiver" }); >> >> The implementation for jsExport: >> >> function jsExport(namespace, components, opt_props) { >> var nsParts = namespace.split("."); >> var base = window; >> var prevBase = null; >> var part = null; >> for (var i=0; part=nsParts.shift();) { >> // export "namespaces" >> base[part]=components[i++]; >> prevBase = base; >> base=base[part]; >> } >> var props = opt_props || {}; >> var exportProps = function(root) { >> for (var prop in props) { if (props.hasOwnProperty(prop)) { >> root[props[prop]] = root[prop]; >> } } >> }; >> if (typeof base === "object") { >> // "singleton" object ie. gadgets.rpc w/ props "call", "setupReceiver" >> etc. >> // just export the symbols immediately >> exportProps(root); >> } else if (typeof base === "function") { >> // "closured" object such as shindig.uri with methods "getQuery", >> "setFragment" etc. >> // create an wrapper to this function that exports any resulting >> properties it returns in an Object >> var exportedFn = function() { >> var result = base.apply(null, arguments); >> if (typeof result === "object") { >> exportProps(result); >> } >> return result; >> }; >> prevBase[part] = exportedFn; >> } >> } >> >> > > Thanks for making this easy. I'm trying this out now in AbstractJsCompiler. > Warning! I wrote this in e-mail, so I guarantee it's not quite right. I'll translate it to a Shindig CL so we can discuss more precisely. We'll merge our work thereafter. > > >> >>> >>>> 2) else if propMap && typeof obj === "function", assume to be a >>>> Closured-style object constructor. >>>> A) do same as #1, only export window["foo"]["bar"] as a function that >>>> in turn exports the properties it has been given, by name. >>>> >>> >>> If I understand this correctly, you want to allow 2 styles to have >>> symbols exported. Does one offer a benefit that the other doesn't? I >>> personally would go after 1 style, that works. Refactoring all to either >>> style is easy. And I personally think the object-style declaration is more >>> obvious -- symbols don't get mapped by some JS execution/helper, less >>> indirections (ie: no return statements). Thoughts? >>> >> >> AFAIK there will always be >1 way to write exported JS since you can have >> singletons and you can have constructed "Objects" with methods on them. >> Exporting singletons is pretty easy in any case. Exporting methods is harder >> when you use property compression. >> >> That's true whether you use Closured or prototyped objects, since methods >> on each are properties. One advantage of the above mechanism is that it >> works equally well for both styles of object. >> >> >>> >>> How about internal objects like rpc.[a|c|t ...], do those need to be >>> quoted still? >>> >> >> With an export method,* nothing* needs to be quoted, now that I think of >> it, unless that "thing" is an API that's used by non-compiled contexts. >> >> So: >> 1) internal property keys need not be quoted, since they're only >> referenced internally and thus obfuscated consistently >> > > Yes, also consistently across features (well, because the compiler knows > all the features at once). > > >> 2) parameter-bag property keys *do* need to be quoted. Toy example: >> suppose we had library Point that has method setCoords(params) which takes { >> x: foo, y: foo }. 'x' and 'y' need to be quoted since external callers will >> pass in param bags with these as keys. >> > > Yes. > > >> 3) rpc.foo is an interesting case. Technically they do not need to be >> quoted, since in any situation both sides of gadgets.rpc communication >> should be using the same bundle and thus consistent obfuscations. However, >> we should probably quote them anyway for consistent debugging, and because >> bundles might be asymmetric yet complementary (one side has "rpc" while the >> other has "rpc:dynamic-height" leading to different obfuscations). >> > > Yes, we've talked about this, rpc.[t|c|...] are used publicly by client > (ie: rpc.f to find source caller). I don't think you intend clients to use > this in the first place, because otherwise, I'd come up with more meaningful > names. > > >> >>> >>> >>>> This whole edifice would allow us to: >>>> 1. Use property compression optimally. >>>> >>> >>> Yes. >>> >>> >>>> 2. Export only "some" symbols externally. >>>> >>> >>> Yes. Currently, "some" is determined by symbols exported in >>> explicitly-listed features. >>> >>> >>>> 3. Export singletons, Closured, and prototyped objects the same way: >>>> <api> >>>> <exports type="js">foo.bar.method1</exports> >>>> <exports type="js">foo.bar.method2</exports> >>>> ... >>>> >>> How do these export prototyped objects? >>> >> >> @see above for current thought on syntax. Everything up to the last "." >> becomes the "root" namespace; all the properties for the same foo.bar prefix >> become the propMap. >> >> wdyt? >> >> > One thought ... can we assume a feature can only export namespace (ie: > everything before the last property) that is common across all, as long as > not on window. This assumption appears to make the code simpler. ie: > I suppose we could impose that a feature export only a single namespace, as a convention. I don't think it's technically necessary though. Regardless what happens, a JS request could come in for "rpc:core.util" and the exports would be generated as if for a single feature that does as you describe below. Interesting idea about the globals feature exporting directly on window w/ "leaf" symbols "gadgets" et al. What about the case where we don't want to generate exports for anything except requested features though? Consider a request for "rpc". In this case, the exports for "globals" wouldn't apply. We'd only export for "rpc", which means we'd have to generate an exports for "gadgets" and "gadgets.rpc" as well, unfortunately. Cheers, John > > A feature cannot do -- > <exports type=js>gadgets.util.makeClosure</exports> > <exports type=js>gadgets.rpc.call</exports> > // unless this translate to 2 jsExport statements, each for gadgets.util > and gadgets.rpc. > // I can't think of a non-global feature that does this. > > But a feature (like globals) can do -- > <exports type=js>gadgets</exports> > <exports type=js>shindig</exports> > // translate to jsExport(null/window, [], { gadgets : "gadgets", shindig : > "shindig }); > > --j >> >> >>> >>> >>>> </api> >>>> >>>> wdyt? >>>> >>>> >>>> http://codereview.appspot.com/4171051/ >>>> >>> >>> Im putting a prototype covering these different cases. >>> >> >> >
Sign in to reply to this message.
On Wed, Feb 16, 2011 at 1:18 PM, John Hjelmstad <fargo@google.com> wrote: > On Wed, Feb 16, 2011 at 11:32 AM, Michael Hermanto <mhermanto@gmail.com>wrote: > >> >> >> On Tue, Feb 15, 2011 at 3:20 PM, John Hjelmstad <fargo@google.com> wrote: >> >>> Michael - >>> >>> Great to get this going. Comments inline. >>> >>> On Mon, Feb 14, 2011 at 6:21 PM, Michael Hermanto <mhermanto@gmail.com>wrote: >>> >>>> >>>> >>>> On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: >>>> >>>>> Looong comment, partially devised from thinking about this for a >>>>> while... >>>>> >>>> >>>> Thanks for starting this thoughtful discussion. >>>> >>>> >>>>> >>>>> >>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>> File features/src/main/javascript/features/rpc/rpc.js (right): >>>>> >>>>> >>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>> features/src/main/javascript/features/rpc/rpc.js:721: >>>>> gadgets.rpc.config >>>>> = function(config) { >>>>> As I've been thinking about this some more, I'm starting to think that >>>>> our best solution is going to be an improved export mechanism. >>>>> >>>>> It helps me to state quite simply what the goals are. >>>>> 1. Enable property obfuscation. >>>>> 2. Enable exports-as-imports. >>>>> >>>> >>>> Please elaborate this one. >>>> >>> >>> Aka "exports-as-externs," the ability to generate an "externs" symbol >>> file (for compilation) from a given JS bundle without doing so manually. For >>> the vast majority of cases this won't even be needed though, since JS using >>> these libs will either be included in the bundle (so the props will compile >>> appropriately) or be written in some other way, such as inlined JS in HTML >>> using gadgets.rpc.call(..) >>> >> >> Is this merely just dumping all the content of <exports type=js> >> directives to a flat file ? I agree that we should only specify this in one >> place (feature.xml for features makes sense), but it'll also introduce >> another "way" to specify externs (something that people need to learn >> again). >> > > Yes and no, to some extent. This is really only useful for external > callers, such as users of this JS within Google who also use Closure > Compiler. > > As you note, feature.xml is where exports will be. What I'd like is to > avoid these users manually writing an externs.js file, and instead referring > to one that's automatically-generated from the feature itself. > Right, we may also need to support param/return annotations to the resulting externs file, for type-checking purpose when the externs file is used to the another compiler. >>> 3. Enable export of only "requested" symbol features to uncompiled >>>>> users. >>>> >>>> >>>> >>>>> As written here, I'm not sure how we'd export gadgets.rpc, either using >>>>> the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any >>>>> other, since we're hiding the gadgets.rpc decl within an anonymous >>>>> closure. >>>>> >>>>> Interestingly, in the runtime-compiled case the existing, unmodified >>>>> mechanism achieves property compression in the ideal fashion: users of >>>>> gadgets.rpc get properly translated >>>>> >>>>> eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will get >>>>> obfuscated to something like: >>>>> g.P = (function() { // "gadgets.rpc = ..." >>>>> stuff >>>>> return { >>>>> Q: function() { /* call(...) method */ }, >>>>> R: function() { /* setupReceiver(...) method */ }, >>>>> }; >>>>> })(); >>>>> >>>>> dynamic-height, being compiled in the same unit, will have: >>>>> gadgets.rpc.call(...) >>>>> ...changed to: >>>>> g.P.Q(...) >>>>> >>>>> ...which works. >>>>> >>>>> >>>> Correct this works. >>>> >>>> But, currently, anything (say a new function X) return {}'ed from >>>> gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if >>>> 1) not referenced from global/window, 2) not part of "requested" symbols, 3) >>>> never used by client code. However, it should be removed for >>>> EXPLICIT_RUN_TIME. >>>> >>> >>> That's true for anything returned like: >>> return { >>> 'symbol': foo, >>> 'another': bar >>> }; >>> >>> I'm actually now agreeing w/ you that property compaction is a very good >>> idea, so we should not quote these. As discussed above, we can turn on >>> property compaction and all JS in the compiled bundle will work >>> appropriately. @see next comment for more. >>> >>> >>>> >>>> >>>>> The main challenge is when someone wants access to g.P.Q by calling >>>>> gadgets.rpc.call. For this, we need a smarter export method. I wonder >>>>> if >>>>> something like this would work: >>>>> >>>> >>>> I don't follow here, the above example is someone calling >>>> gadgets.rpc.call() and it translates fine to g.P.Q, no? >>>> >>> >>> Yes. I should have stated more clearly -- the main challenge is when an >>> "external" caller (not in the JS bundle) wants to reference g.P.Q by its >>> exported name, gadgets.rpc.call. >>> >>> >>>> >>>> jsExport(obj, namespace, propMap); >>>>> >>>>> For gadgets.rpc, we'd generate: >>>>> >>>>> jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: >>>>> "setupReceiver", ... }); >>>>> >>>>> When compiled, this turns into: >>>>> yZ(g.P, { Q: "call", R: "setupReceiver" }); >>>>> >>>>> The jsExport method would then: >>>>> 1) if propMap && typeof obj === "object", >>>>> A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the >>>>> given symbol on window. >>>>> B) export properties directly by name, as singletons. Iterate through >>>>> propMap, exporting properties by name, eg. >>>>> window["gadgets"]["rpc"]["call"] = g.P["Q"]; >>>>> >>>> >>>> Who's generating these jsExport(...) statements ? The JS serving >>>> mechanism based on current <exports> or JS developer ? >>>> >>> >>> JS serving mechanism. The difference is that instead of exporting >>> directly on window, it delegates to a "smart" JS function to do so. >>> >>> So if your <exports> are: >>> gadgets.flash.embedFlash >>> gadgets.rpc.call >>> gadgets.rpc.setupReceiver >>> >>> ...your generated exports are currently something like this: >>> window["gadgets"]=gadgets; >>> window["gadgets"]["rpc"]=gadgets.rpc; >>> window["gadgets"]["rpc"]["call"]=gadgets.rpc.call; >>> etc etc >>> >>> With the help of a smart export function, we can make this both richer >>> (supporting exported closured-Objects) and more compact. The minimal would >>> be: >>> jsExport("gadgets.flash", [ gadgets, gadgets.flash, gadgets.flash ], { >>> embedFlash: "embedFlash" }); >>> jsExport("gadgets.rpc", [ gadgets, gadgets.rpc ], { call: "call", >>> setupReceiver: "setupReceiver" }); >>> >>> The implementation for jsExport: >>> >>> function jsExport(namespace, components, opt_props) { >>> var nsParts = namespace.split("."); >>> var base = window; >>> var prevBase = null; >>> var part = null; >>> for (var i=0; part=nsParts.shift();) { >>> // export "namespaces" >>> base[part]=components[i++]; >>> prevBase = base; >>> base=base[part]; >>> } >>> var props = opt_props || {}; >>> var exportProps = function(root) { >>> for (var prop in props) { if (props.hasOwnProperty(prop)) { >>> root[props[prop]] = root[prop]; >>> } } >>> }; >>> if (typeof base === "object") { >>> // "singleton" object ie. gadgets.rpc w/ props "call", >>> "setupReceiver" etc. >>> // just export the symbols immediately >>> exportProps(root); >>> } else if (typeof base === "function") { >>> // "closured" object such as shindig.uri with methods "getQuery", >>> "setFragment" etc. >>> // create an wrapper to this function that exports any resulting >>> properties it returns in an Object >>> var exportedFn = function() { >>> var result = base.apply(null, arguments); >>> if (typeof result === "object") { >>> exportProps(result); >>> } >>> return result; >>> }; >>> prevBase[part] = exportedFn; >>> } >>> } >>> >>> >> >> Thanks for making this easy. I'm trying this out now in >> AbstractJsCompiler. >> > > Warning! I wrote this in e-mail, so I guarantee it's not quite right. I'll > translate it to a Shindig CL so we can discuss more precisely. We'll merge > our work thereafter. > As talked offline, I'm already on this. > > >> >> >>> >>>> >>>>> 2) else if propMap && typeof obj === "function", assume to be a >>>>> Closured-style object constructor. >>>>> A) do same as #1, only export window["foo"]["bar"] as a function that >>>>> in turn exports the properties it has been given, by name. >>>>> >>>> >>>> If I understand this correctly, you want to allow 2 styles to have >>>> symbols exported. Does one offer a benefit that the other doesn't? I >>>> personally would go after 1 style, that works. Refactoring all to either >>>> style is easy. And I personally think the object-style declaration is more >>>> obvious -- symbols don't get mapped by some JS execution/helper, less >>>> indirections (ie: no return statements). Thoughts? >>>> >>> >>> AFAIK there will always be >1 way to write exported JS since you can have >>> singletons and you can have constructed "Objects" with methods on them. >>> Exporting singletons is pretty easy in any case. Exporting methods is harder >>> when you use property compression. >>> >>> That's true whether you use Closured or prototyped objects, since methods >>> on each are properties. One advantage of the above mechanism is that it >>> works equally well for both styles of object. >>> >>> >>>> >>>> How about internal objects like rpc.[a|c|t ...], do those need to be >>>> quoted still? >>>> >>> >>> With an export method,* nothing* needs to be quoted, now that I think of >>> it, unless that "thing" is an API that's used by non-compiled contexts. >>> >>> So: >>> 1) internal property keys need not be quoted, since they're only >>> referenced internally and thus obfuscated consistently >>> >> >> Yes, also consistently across features (well, because the compiler knows >> all the features at once). >> >> >>> 2) parameter-bag property keys *do* need to be quoted. Toy example: >>> suppose we had library Point that has method setCoords(params) which takes { >>> x: foo, y: foo }. 'x' and 'y' need to be quoted since external callers will >>> pass in param bags with these as keys. >>> >> >> Yes. >> >> >>> 3) rpc.foo is an interesting case. Technically they do not need to be >>> quoted, since in any situation both sides of gadgets.rpc communication >>> should be using the same bundle and thus consistent obfuscations. However, >>> we should probably quote them anyway for consistent debugging, and because >>> bundles might be asymmetric yet complementary (one side has "rpc" while the >>> other has "rpc:dynamic-height" leading to different obfuscations). >>> >> >> Yes, we've talked about this, rpc.[t|c|...] are used publicly by client >> (ie: rpc.f to find source caller). I don't think you intend clients to use >> this in the first place, because otherwise, I'd come up with more meaningful >> names. >> >> >>> >>>> >>>> >>>>> This whole edifice would allow us to: >>>>> 1. Use property compression optimally. >>>>> >>>> >>>> Yes. >>>> >>>> >>>>> 2. Export only "some" symbols externally. >>>>> >>>> >>>> Yes. Currently, "some" is determined by symbols exported in >>>> explicitly-listed features. >>>> >>>> >>>>> 3. Export singletons, Closured, and prototyped objects the same way: >>>>> <api> >>>>> <exports type="js">foo.bar.method1</exports> >>>>> <exports type="js">foo.bar.method2</exports> >>>>> ... >>>>> >>>> How do these export prototyped objects? >>>> >>> >>> @see above for current thought on syntax. Everything up to the last "." >>> becomes the "root" namespace; all the properties for the same foo.bar prefix >>> become the propMap. >>> >>> wdyt? >>> >>> >> One thought ... can we assume a feature can only export namespace (ie: >> everything before the last property) that is common across all, as long as >> not on window. This assumption appears to make the code simpler. ie: >> > > I suppose we could impose that a feature export only a single namespace, as > a convention. I don't think it's technically necessary though. Regardless > what happens, a JS request could come in for "rpc:core.util" and the exports > would be generated as if for a single feature that does as you describe > below. > > Interesting idea about the globals feature exporting directly on window w/ > "leaf" symbols "gadgets" et al. > What about the case where we don't want to generate exports for anything > except requested features though? > > Consider a request for "rpc". In this case, the exports for "globals" > wouldn't apply. We'd only export for "rpc", which means we'd have to > generate an exports for "gadgets" and "gadgets.rpc" as well, unfortunately. > Hm, this is tricky. I see two types of dependencies here -- 1) deps by namespace (what you described), 2) deps by functionalities (in feature.xml <dependency> sense). For 1), I'm incline to say to not inline exportJs() statements for globals, but exportJs will be smart enough to make "gadgets" namespace available if not exist. > Cheers, > John > > >> >> A feature cannot do -- >> <exports type=js>gadgets.util.makeClosure</exports> >> <exports type=js>gadgets.rpc.call</exports> >> // unless this translate to 2 jsExport statements, each for gadgets.util >> and gadgets.rpc. >> // I can't think of a non-global feature that does this. >> >> But a feature (like globals) can do -- >> <exports type=js>gadgets</exports> >> <exports type=js>shindig</exports> >> // translate to jsExport(null/window, [], { gadgets : "gadgets", shindig : >> "shindig }); >> >> --j >>> >>> >>>> >>>> >>>>> </api> >>>>> >>>>> wdyt? >>>>> >>>>> >>>>> http://codereview.appspot.com/4171051/ >>>>> >>>> >>>> Im putting a prototype covering these different cases. >>>> >>> >>> >> >
Sign in to reply to this message.
On Wed, Feb 16, 2011 at 1:30 PM, Michael Hermanto <mhermanto@gmail.com>wrote: > > > On Wed, Feb 16, 2011 at 1:18 PM, John Hjelmstad <fargo@google.com> wrote: > >> On Wed, Feb 16, 2011 at 11:32 AM, Michael Hermanto <mhermanto@gmail.com>wrote: >> >>> >>> >>> On Tue, Feb 15, 2011 at 3:20 PM, John Hjelmstad <fargo@google.com>wrote: >>> >>>> Michael - >>>> >>>> Great to get this going. Comments inline. >>>> >>>> On Mon, Feb 14, 2011 at 6:21 PM, Michael Hermanto <mhermanto@gmail.com>wrote: >>>> >>>>> >>>>> >>>>> On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: >>>>> >>>>>> Looong comment, partially devised from thinking about this for a >>>>>> while... >>>>>> >>>>> >>>>> Thanks for starting this thoughtful discussion. >>>>> >>>>> >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>>> File features/src/main/javascript/features/rpc/rpc.js (right): >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>>> features/src/main/javascript/features/rpc/rpc.js:721: >>>>>> gadgets.rpc.config >>>>>> = function(config) { >>>>>> As I've been thinking about this some more, I'm starting to think that >>>>>> our best solution is going to be an improved export mechanism. >>>>>> >>>>>> It helps me to state quite simply what the goals are. >>>>>> 1. Enable property obfuscation. >>>>>> 2. Enable exports-as-imports. >>>>>> >>>>> >>>>> Please elaborate this one. >>>>> >>>> >>>> Aka "exports-as-externs," the ability to generate an "externs" symbol >>>> file (for compilation) from a given JS bundle without doing so manually. For >>>> the vast majority of cases this won't even be needed though, since JS using >>>> these libs will either be included in the bundle (so the props will compile >>>> appropriately) or be written in some other way, such as inlined JS in HTML >>>> using gadgets.rpc.call(..) >>>> >>> >>> Is this merely just dumping all the content of <exports type=js> >>> directives to a flat file ? I agree that we should only specify this in one >>> place (feature.xml for features makes sense), but it'll also introduce >>> another "way" to specify externs (something that people need to learn >>> again). >>> >> >> Yes and no, to some extent. This is really only useful for external >> callers, such as users of this JS within Google who also use Closure >> Compiler. >> >> As you note, feature.xml is where exports will be. What I'd like is to >> avoid these users manually writing an externs.js file, and instead referring >> to one that's automatically-generated from the feature itself. >> > > Right, we may also need to support param/return annotations to the > resulting externs file, for type-checking purpose when the externs file is > used to the another compiler. > Yep, I think Closure Compiler actually does this (?). > > >>>> 3. Enable export of only "requested" symbol features to uncompiled >>>>>> users. >>>>> >>>>> >>>>> >>>>>> As written here, I'm not sure how we'd export gadgets.rpc, either >>>>>> using >>>>>> the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any >>>>>> other, since we're hiding the gadgets.rpc decl within an anonymous >>>>>> closure. >>>>>> >>>>>> Interestingly, in the runtime-compiled case the existing, unmodified >>>>>> mechanism achieves property compression in the ideal fashion: users of >>>>>> gadgets.rpc get properly translated >>>>>> >>>>>> eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will get >>>>>> obfuscated to something like: >>>>>> g.P = (function() { // "gadgets.rpc = ..." >>>>>> stuff >>>>>> return { >>>>>> Q: function() { /* call(...) method */ }, >>>>>> R: function() { /* setupReceiver(...) method */ }, >>>>>> }; >>>>>> })(); >>>>>> >>>>>> dynamic-height, being compiled in the same unit, will have: >>>>>> gadgets.rpc.call(...) >>>>>> ...changed to: >>>>>> g.P.Q(...) >>>>>> >>>>>> ...which works. >>>>>> >>>>>> >>>>> Correct this works. >>>>> >>>>> But, currently, anything (say a new function X) return {}'ed from >>>>> gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if >>>>> 1) not referenced from global/window, 2) not part of "requested" symbols, 3) >>>>> never used by client code. However, it should be removed for >>>>> EXPLICIT_RUN_TIME. >>>>> >>>> >>>> That's true for anything returned like: >>>> return { >>>> 'symbol': foo, >>>> 'another': bar >>>> }; >>>> >>>> I'm actually now agreeing w/ you that property compaction is a very good >>>> idea, so we should not quote these. As discussed above, we can turn on >>>> property compaction and all JS in the compiled bundle will work >>>> appropriately. @see next comment for more. >>>> >>>> >>>>> >>>>> >>>>>> The main challenge is when someone wants access to g.P.Q by calling >>>>>> gadgets.rpc.call. For this, we need a smarter export method. I wonder >>>>>> if >>>>>> something like this would work: >>>>>> >>>>> >>>>> I don't follow here, the above example is someone calling >>>>> gadgets.rpc.call() and it translates fine to g.P.Q, no? >>>>> >>>> >>>> Yes. I should have stated more clearly -- the main challenge is when an >>>> "external" caller (not in the JS bundle) wants to reference g.P.Q by its >>>> exported name, gadgets.rpc.call. >>>> >>>> >>>>> >>>>> jsExport(obj, namespace, propMap); >>>>>> >>>>>> For gadgets.rpc, we'd generate: >>>>>> >>>>>> jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: >>>>>> "setupReceiver", ... }); >>>>>> >>>>>> When compiled, this turns into: >>>>>> yZ(g.P, { Q: "call", R: "setupReceiver" }); >>>>>> >>>>>> The jsExport method would then: >>>>>> 1) if propMap && typeof obj === "object", >>>>>> A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the >>>>>> given symbol on window. >>>>>> B) export properties directly by name, as singletons. Iterate through >>>>>> propMap, exporting properties by name, eg. >>>>>> window["gadgets"]["rpc"]["call"] = g.P["Q"]; >>>>>> >>>>> >>>>> Who's generating these jsExport(...) statements ? The JS serving >>>>> mechanism based on current <exports> or JS developer ? >>>>> >>>> >>>> JS serving mechanism. The difference is that instead of exporting >>>> directly on window, it delegates to a "smart" JS function to do so. >>>> >>>> So if your <exports> are: >>>> gadgets.flash.embedFlash >>>> gadgets.rpc.call >>>> gadgets.rpc.setupReceiver >>>> >>>> ...your generated exports are currently something like this: >>>> window["gadgets"]=gadgets; >>>> window["gadgets"]["rpc"]=gadgets.rpc; >>>> window["gadgets"]["rpc"]["call"]=gadgets.rpc.call; >>>> etc etc >>>> >>>> With the help of a smart export function, we can make this both richer >>>> (supporting exported closured-Objects) and more compact. The minimal would >>>> be: >>>> jsExport("gadgets.flash", [ gadgets, gadgets.flash, gadgets.flash ], { >>>> embedFlash: "embedFlash" }); >>>> jsExport("gadgets.rpc", [ gadgets, gadgets.rpc ], { call: "call", >>>> setupReceiver: "setupReceiver" }); >>>> >>>> The implementation for jsExport: >>>> >>>> function jsExport(namespace, components, opt_props) { >>>> var nsParts = namespace.split("."); >>>> var base = window; >>>> var prevBase = null; >>>> var part = null; >>>> for (var i=0; part=nsParts.shift();) { >>>> // export "namespaces" >>>> base[part]=components[i++]; >>>> prevBase = base; >>>> base=base[part]; >>>> } >>>> var props = opt_props || {}; >>>> var exportProps = function(root) { >>>> for (var prop in props) { if (props.hasOwnProperty(prop)) { >>>> root[props[prop]] = root[prop]; >>>> } } >>>> }; >>>> if (typeof base === "object") { >>>> // "singleton" object ie. gadgets.rpc w/ props "call", >>>> "setupReceiver" etc. >>>> // just export the symbols immediately >>>> exportProps(root); >>>> } else if (typeof base === "function") { >>>> // "closured" object such as shindig.uri with methods "getQuery", >>>> "setFragment" etc. >>>> // create an wrapper to this function that exports any resulting >>>> properties it returns in an Object >>>> var exportedFn = function() { >>>> var result = base.apply(null, arguments); >>>> if (typeof result === "object") { >>>> exportProps(result); >>>> } >>>> return result; >>>> }; >>>> prevBase[part] = exportedFn; >>>> } >>>> } >>>> >>>> >>> >>> Thanks for making this easy. I'm trying this out now in >>> AbstractJsCompiler. >>> >> >> Warning! I wrote this in e-mail, so I guarantee it's not quite right. I'll >> translate it to a Shindig CL so we can discuss more precisely. We'll merge >> our work thereafter. >> > > As talked offline, I'm already on this. > Will not duplicate effort :) > >> >>> >>> >>>> >>>>> >>>>>> 2) else if propMap && typeof obj === "function", assume to be a >>>>>> Closured-style object constructor. >>>>>> A) do same as #1, only export window["foo"]["bar"] as a function that >>>>>> in turn exports the properties it has been given, by name. >>>>>> >>>>> >>>>> If I understand this correctly, you want to allow 2 styles to have >>>>> symbols exported. Does one offer a benefit that the other doesn't? I >>>>> personally would go after 1 style, that works. Refactoring all to either >>>>> style is easy. And I personally think the object-style declaration is more >>>>> obvious -- symbols don't get mapped by some JS execution/helper, less >>>>> indirections (ie: no return statements). Thoughts? >>>>> >>>> >>>> AFAIK there will always be >1 way to write exported JS since you can >>>> have singletons and you can have constructed "Objects" with methods on them. >>>> Exporting singletons is pretty easy in any case. Exporting methods is harder >>>> when you use property compression. >>>> >>>> That's true whether you use Closured or prototyped objects, since >>>> methods on each are properties. One advantage of the above mechanism is that >>>> it works equally well for both styles of object. >>>> >>>> >>>>> >>>>> How about internal objects like rpc.[a|c|t ...], do those need to be >>>>> quoted still? >>>>> >>>> >>>> With an export method,* nothing* needs to be quoted, now that I think >>>> of it, unless that "thing" is an API that's used by non-compiled contexts. >>>> >>>> So: >>>> 1) internal property keys need not be quoted, since they're only >>>> referenced internally and thus obfuscated consistently >>>> >>> >>> Yes, also consistently across features (well, because the compiler knows >>> all the features at once). >>> >>> >>>> 2) parameter-bag property keys *do* need to be quoted. Toy example: >>>> suppose we had library Point that has method setCoords(params) which takes { >>>> x: foo, y: foo }. 'x' and 'y' need to be quoted since external callers will >>>> pass in param bags with these as keys. >>>> >>> >>> Yes. >>> >>> >>>> 3) rpc.foo is an interesting case. Technically they do not need to be >>>> quoted, since in any situation both sides of gadgets.rpc communication >>>> should be using the same bundle and thus consistent obfuscations. However, >>>> we should probably quote them anyway for consistent debugging, and because >>>> bundles might be asymmetric yet complementary (one side has "rpc" while the >>>> other has "rpc:dynamic-height" leading to different obfuscations). >>>> >>> >>> Yes, we've talked about this, rpc.[t|c|...] are used publicly by client >>> (ie: rpc.f to find source caller). I don't think you intend clients to use >>> this in the first place, because otherwise, I'd come up with more meaningful >>> names. >>> >>> >>>> >>>>> >>>>> >>>>>> This whole edifice would allow us to: >>>>>> 1. Use property compression optimally. >>>>>> >>>>> >>>>> Yes. >>>>> >>>>> >>>>>> 2. Export only "some" symbols externally. >>>>>> >>>>> >>>>> Yes. Currently, "some" is determined by symbols exported in >>>>> explicitly-listed features. >>>>> >>>>> >>>>>> 3. Export singletons, Closured, and prototyped objects the same way: >>>>>> <api> >>>>>> <exports type="js">foo.bar.method1</exports> >>>>>> <exports type="js">foo.bar.method2</exports> >>>>>> ... >>>>>> >>>>> How do these export prototyped objects? >>>>> >>>> >>>> @see above for current thought on syntax. Everything up to the last "." >>>> becomes the "root" namespace; all the properties for the same foo.bar prefix >>>> become the propMap. >>>> >>>> wdyt? >>>> >>>> >>> One thought ... can we assume a feature can only export namespace (ie: >>> everything before the last property) that is common across all, as long as >>> not on window. This assumption appears to make the code simpler. ie: >>> >> >> I suppose we could impose that a feature export only a single namespace, >> as a convention. I don't think it's technically necessary though. Regardless >> what happens, a JS request could come in for "rpc:core.util" and the exports >> would be generated as if for a single feature that does as you describe >> below. >> >> Interesting idea about the globals feature exporting directly on window w/ >> "leaf" symbols "gadgets" et al. >> > What about the case where we don't want to generate exports for anything >> except requested features though? >> > >> Consider a request for "rpc". In this case, the exports for "globals" >> wouldn't apply. We'd only export for "rpc", which means we'd have to >> generate an exports for "gadgets" and "gadgets.rpc" as well, unfortunately. >> > > Hm, this is tricky. I see two types of dependencies here -- 1) deps by > namespace (what you described), 2) deps by functionalities (in feature.xml > <dependency> sense). For 1), I'm incline to say to not inline exportJs() > statements for globals, but exportJs will be smart enough to make "gadgets" > namespace available if not exist. > Agree. > > >> Cheers, >> John >> >> >>> >>> A feature cannot do -- >>> <exports type=js>gadgets.util.makeClosure</exports> >>> <exports type=js>gadgets.rpc.call</exports> >>> // unless this translate to 2 jsExport statements, each for gadgets.util >>> and gadgets.rpc. >>> // I can't think of a non-global feature that does this. >>> >>> But a feature (like globals) can do -- >>> <exports type=js>gadgets</exports> >>> <exports type=js>shindig</exports> >>> // translate to jsExport(null/window, [], { gadgets : "gadgets", shindig >>> : "shindig }); >>> >>> --j >>>> >>>> >>>>> >>>>> >>>>>> </api> >>>>>> >>>>>> wdyt? >>>>>> >>>>>> >>>>>> http://codereview.appspot.com/4171051/ >>>>>> >>>>> >>>>> Im putting a prototype covering these different cases. >>>>> >>>> >>>> >>> >> >
Sign in to reply to this message.
Update patch
Sign in to reply to this message.
On Wed, Feb 16, 2011 at 1:32 PM, John Hjelmstad <fargo@google.com> wrote: > On Wed, Feb 16, 2011 at 1:30 PM, Michael Hermanto <mhermanto@gmail.com>wrote: > >> >> >> On Wed, Feb 16, 2011 at 1:18 PM, John Hjelmstad <fargo@google.com> wrote: >> >>> On Wed, Feb 16, 2011 at 11:32 AM, Michael Hermanto <mhermanto@gmail.com>wrote: >>> >>>> >>>> >>>> On Tue, Feb 15, 2011 at 3:20 PM, John Hjelmstad <fargo@google.com>wrote: >>>> >>>>> Michael - >>>>> >>>>> Great to get this going. Comments inline. >>>>> >>>>> On Mon, Feb 14, 2011 at 6:21 PM, Michael Hermanto <mhermanto@gmail.com >>>>> > wrote: >>>>> >>>>>> >>>>>> >>>>>> On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: >>>>>> >>>>>>> Looong comment, partially devised from thinking about this for a >>>>>>> while... >>>>>>> >>>>>> >>>>>> Thanks for starting this thoughtful discussion. >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>>>> File features/src/main/javascript/features/rpc/rpc.js (right): >>>>>>> >>>>>>> >>>>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>>>> features/src/main/javascript/features/rpc/rpc.js:721: >>>>>>> gadgets.rpc.config >>>>>>> = function(config) { >>>>>>> As I've been thinking about this some more, I'm starting to think >>>>>>> that >>>>>>> our best solution is going to be an improved export mechanism. >>>>>>> >>>>>>> It helps me to state quite simply what the goals are. >>>>>>> 1. Enable property obfuscation. >>>>>>> 2. Enable exports-as-imports. >>>>>>> >>>>>> >>>>>> Please elaborate this one. >>>>>> >>>>> >>>>> Aka "exports-as-externs," the ability to generate an "externs" symbol >>>>> file (for compilation) from a given JS bundle without doing so manually. For >>>>> the vast majority of cases this won't even be needed though, since JS using >>>>> these libs will either be included in the bundle (so the props will compile >>>>> appropriately) or be written in some other way, such as inlined JS in HTML >>>>> using gadgets.rpc.call(..) >>>>> >>>> >>>> Is this merely just dumping all the content of <exports type=js> >>>> directives to a flat file ? I agree that we should only specify this in one >>>> place (feature.xml for features makes sense), but it'll also introduce >>>> another "way" to specify externs (something that people need to learn >>>> again). >>>> >>> >>> Yes and no, to some extent. This is really only useful for external >>> callers, such as users of this JS within Google who also use Closure >>> Compiler. >>> >>> As you note, feature.xml is where exports will be. What I'd like is to >>> avoid these users manually writing an externs.js file, and instead referring >>> to one that's automatically-generated from the feature itself. >>> >> >> Right, we may also need to support param/return annotations to the >> resulting externs file, for type-checking purpose when the externs file is >> used to the another compiler. >> > > Yep, I think Closure Compiler actually does this (?). > > >> >> >>>>> 3. Enable export of only "requested" symbol features to uncompiled >>>>>>> users. >>>>>> >>>>>> >>>>>> >>>>>>> As written here, I'm not sure how we'd export gadgets.rpc, either >>>>>>> using >>>>>>> the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any >>>>>>> other, since we're hiding the gadgets.rpc decl within an anonymous >>>>>>> closure. >>>>>>> >>>>>>> Interestingly, in the runtime-compiled case the existing, unmodified >>>>>>> mechanism achieves property compression in the ideal fashion: users >>>>>>> of >>>>>>> gadgets.rpc get properly translated >>>>>>> >>>>>>> eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will >>>>>>> get >>>>>>> obfuscated to something like: >>>>>>> g.P = (function() { // "gadgets.rpc = ..." >>>>>>> stuff >>>>>>> return { >>>>>>> Q: function() { /* call(...) method */ }, >>>>>>> R: function() { /* setupReceiver(...) method */ }, >>>>>>> }; >>>>>>> })(); >>>>>>> >>>>>>> dynamic-height, being compiled in the same unit, will have: >>>>>>> gadgets.rpc.call(...) >>>>>>> ...changed to: >>>>>>> g.P.Q(...) >>>>>>> >>>>>>> ...which works. >>>>>>> >>>>>>> >>>>>> Correct this works. >>>>>> >>>>>> But, currently, anything (say a new function X) return {}'ed from >>>>>> gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if >>>>>> 1) not referenced from global/window, 2) not part of "requested" symbols, 3) >>>>>> never used by client code. However, it should be removed for >>>>>> EXPLICIT_RUN_TIME. >>>>>> >>>>> >>>>> That's true for anything returned like: >>>>> return { >>>>> 'symbol': foo, >>>>> 'another': bar >>>>> }; >>>>> >>>>> I'm actually now agreeing w/ you that property compaction is a very >>>>> good idea, so we should not quote these. As discussed above, we can turn on >>>>> property compaction and all JS in the compiled bundle will work >>>>> appropriately. @see next comment for more. >>>>> >>>>> >>>>>> >>>>>> >>>>>>> The main challenge is when someone wants access to g.P.Q by calling >>>>>>> gadgets.rpc.call. For this, we need a smarter export method. I wonder >>>>>>> if >>>>>>> something like this would work: >>>>>>> >>>>>> >>>>>> I don't follow here, the above example is someone calling >>>>>> gadgets.rpc.call() and it translates fine to g.P.Q, no? >>>>>> >>>>> >>>>> Yes. I should have stated more clearly -- the main challenge is when an >>>>> "external" caller (not in the JS bundle) wants to reference g.P.Q by its >>>>> exported name, gadgets.rpc.call. >>>>> >>>>> >>>>>> >>>>>> jsExport(obj, namespace, propMap); >>>>>>> >>>>>>> For gadgets.rpc, we'd generate: >>>>>>> >>>>>>> jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: >>>>>>> "setupReceiver", ... }); >>>>>>> >>>>>>> When compiled, this turns into: >>>>>>> yZ(g.P, { Q: "call", R: "setupReceiver" }); >>>>>>> >>>>>>> The jsExport method would then: >>>>>>> 1) if propMap && typeof obj === "object", >>>>>>> A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the >>>>>>> given symbol on window. >>>>>>> B) export properties directly by name, as singletons. Iterate >>>>>>> through >>>>>>> propMap, exporting properties by name, eg. >>>>>>> window["gadgets"]["rpc"]["call"] = g.P["Q"]; >>>>>>> >>>>>> >>>>>> Who's generating these jsExport(...) statements ? The JS serving >>>>>> mechanism based on current <exports> or JS developer ? >>>>>> >>>>> >>>>> JS serving mechanism. The difference is that instead of exporting >>>>> directly on window, it delegates to a "smart" JS function to do so. >>>>> >>>>> So if your <exports> are: >>>>> gadgets.flash.embedFlash >>>>> gadgets.rpc.call >>>>> gadgets.rpc.setupReceiver >>>>> >>>>> ...your generated exports are currently something like this: >>>>> window["gadgets"]=gadgets; >>>>> window["gadgets"]["rpc"]=gadgets.rpc; >>>>> window["gadgets"]["rpc"]["call"]=gadgets.rpc.call; >>>>> etc etc >>>>> >>>>> With the help of a smart export function, we can make this both richer >>>>> (supporting exported closured-Objects) and more compact. The minimal would >>>>> be: >>>>> jsExport("gadgets.flash", [ gadgets, gadgets.flash, gadgets.flash ], { >>>>> embedFlash: "embedFlash" }); >>>>> jsExport("gadgets.rpc", [ gadgets, gadgets.rpc ], { call: "call", >>>>> setupReceiver: "setupReceiver" }); >>>>> >>>>> The implementation for jsExport: >>>>> >>>>> function jsExport(namespace, components, opt_props) { >>>>> var nsParts = namespace.split("."); >>>>> var base = window; >>>>> var prevBase = null; >>>>> var part = null; >>>>> for (var i=0; part=nsParts.shift();) { >>>>> // export "namespaces" >>>>> base[part]=components[i++]; >>>>> prevBase = base; >>>>> base=base[part]; >>>>> } >>>>> var props = opt_props || {}; >>>>> var exportProps = function(root) { >>>>> for (var prop in props) { if (props.hasOwnProperty(prop)) { >>>>> root[props[prop]] = root[prop]; >>>>> } } >>>>> }; >>>>> if (typeof base === "object") { >>>>> // "singleton" object ie. gadgets.rpc w/ props "call", >>>>> "setupReceiver" etc. >>>>> // just export the symbols immediately >>>>> exportProps(root); >>>>> } else if (typeof base === "function") { >>>>> // "closured" object such as shindig.uri with methods "getQuery", >>>>> "setFragment" etc. >>>>> // create an wrapper to this function that exports any resulting >>>>> properties it returns in an Object >>>>> var exportedFn = function() { >>>>> var result = base.apply(null, arguments); >>>>> if (typeof result === "object") { >>>>> exportProps(result); >>>>> } >>>>> return result; >>>>> }; >>>>> prevBase[part] = exportedFn; >>>>> } >>>>> } >>>>> >>>>> >>>> >>>> Thanks for making this easy. I'm trying this out now in >>>> AbstractJsCompiler. >>>> >>> >>> Warning! I wrote this in e-mail, so I guarantee it's not quite right. >>> I'll translate it to a Shindig CL so we can discuss more precisely. We'll >>> merge our work thereafter. >>> >> >> As talked offline, I'm already on this. >> > > Will not duplicate effort :) > Uploaded new patch. Tested to work across all export cases (as Javadoc'ed), including prototype'd properties. PTAL. > >> >>> >>>> >>>> >>>>> >>>>>> >>>>>>> 2) else if propMap && typeof obj === "function", assume to be a >>>>>>> Closured-style object constructor. >>>>>>> A) do same as #1, only export window["foo"]["bar"] as a function >>>>>>> that >>>>>>> in turn exports the properties it has been given, by name. >>>>>>> >>>>>> >>>>>> If I understand this correctly, you want to allow 2 styles to have >>>>>> symbols exported. Does one offer a benefit that the other doesn't? I >>>>>> personally would go after 1 style, that works. Refactoring all to either >>>>>> style is easy. And I personally think the object-style declaration is more >>>>>> obvious -- symbols don't get mapped by some JS execution/helper, less >>>>>> indirections (ie: no return statements). Thoughts? >>>>>> >>>>> >>>>> AFAIK there will always be >1 way to write exported JS since you can >>>>> have singletons and you can have constructed "Objects" with methods on them. >>>>> Exporting singletons is pretty easy in any case. Exporting methods is harder >>>>> when you use property compression. >>>>> >>>>> That's true whether you use Closured or prototyped objects, since >>>>> methods on each are properties. One advantage of the above mechanism is that >>>>> it works equally well for both styles of object. >>>>> >>>>> >>>>>> >>>>>> How about internal objects like rpc.[a|c|t ...], do those need to be >>>>>> quoted still? >>>>>> >>>>> >>>>> With an export method,* nothing* needs to be quoted, now that I think >>>>> of it, unless that "thing" is an API that's used by non-compiled contexts. >>>>> >>>>> So: >>>>> 1) internal property keys need not be quoted, since they're only >>>>> referenced internally and thus obfuscated consistently >>>>> >>>> >>>> Yes, also consistently across features (well, because the compiler knows >>>> all the features at once). >>>> >>>> >>>>> 2) parameter-bag property keys *do* need to be quoted. Toy example: >>>>> suppose we had library Point that has method setCoords(params) which takes { >>>>> x: foo, y: foo }. 'x' and 'y' need to be quoted since external callers will >>>>> pass in param bags with these as keys. >>>>> >>>> >>>> Yes. >>>> >>>> >>>>> 3) rpc.foo is an interesting case. Technically they do not need to be >>>>> quoted, since in any situation both sides of gadgets.rpc communication >>>>> should be using the same bundle and thus consistent obfuscations. However, >>>>> we should probably quote them anyway for consistent debugging, and because >>>>> bundles might be asymmetric yet complementary (one side has "rpc" while the >>>>> other has "rpc:dynamic-height" leading to different obfuscations). >>>>> >>>> >>>> Yes, we've talked about this, rpc.[t|c|...] are used publicly by client >>>> (ie: rpc.f to find source caller). I don't think you intend clients to use >>>> this in the first place, because otherwise, I'd come up with more meaningful >>>> names. >>>> >>>> >>>>> >>>>>> >>>>>> >>>>>>> This whole edifice would allow us to: >>>>>>> 1. Use property compression optimally. >>>>>>> >>>>>> >>>>>> Yes. >>>>>> >>>>>> >>>>>>> 2. Export only "some" symbols externally. >>>>>>> >>>>>> >>>>>> Yes. Currently, "some" is determined by symbols exported in >>>>>> explicitly-listed features. >>>>>> >>>>>> >>>>>>> 3. Export singletons, Closured, and prototyped objects the same way: >>>>>>> <api> >>>>>>> <exports type="js">foo.bar.method1</exports> >>>>>>> <exports type="js">foo.bar.method2</exports> >>>>>>> ... >>>>>>> >>>>>> How do these export prototyped objects? >>>>>> >>>>> >>>>> @see above for current thought on syntax. Everything up to the last "." >>>>> becomes the "root" namespace; all the properties for the same foo.bar prefix >>>>> become the propMap. >>>>> >>>>> wdyt? >>>>> >>>>> >>>> One thought ... can we assume a feature can only export namespace (ie: >>>> everything before the last property) that is common across all, as long as >>>> not on window. This assumption appears to make the code simpler. ie: >>>> >>> >>> I suppose we could impose that a feature export only a single namespace, >>> as a convention. I don't think it's technically necessary though. Regardless >>> what happens, a JS request could come in for "rpc:core.util" and the exports >>> would be generated as if for a single feature that does as you describe >>> below. >>> >>> Interesting idea about the globals feature exporting directly on window >>> w/ "leaf" symbols "gadgets" et al. >>> >> What about the case where we don't want to generate exports for anything >>> except requested features though? >>> >> >>> Consider a request for "rpc". In this case, the exports for "globals" >>> wouldn't apply. We'd only export for "rpc", which means we'd have to >>> generate an exports for "gadgets" and "gadgets.rpc" as well, unfortunately. >>> >> >> Hm, this is tricky. I see two types of dependencies here -- 1) deps by >> namespace (what you described), 2) deps by functionalities (in feature.xml >> <dependency> sense). For 1), I'm incline to say to not inline exportJs() >> statements for globals, but exportJs will be smart enough to make "gadgets" >> namespace available if not exist. >> > > Agree. > > >> >> >>> Cheers, >>> John >>> >>> >>>> >>>> A feature cannot do -- >>>> <exports type=js>gadgets.util.makeClosure</exports> >>>> <exports type=js>gadgets.rpc.call</exports> >>>> // unless this translate to 2 jsExport statements, each for gadgets.util >>>> and gadgets.rpc. >>>> // I can't think of a non-global feature that does this. >>>> >>>> But a feature (like globals) can do -- >>>> <exports type=js>gadgets</exports> >>>> <exports type=js>shindig</exports> >>>> // translate to jsExport(null/window, [], { gadgets : "gadgets", shindig >>>> : "shindig }); >>>> >>>> --j >>>>> >>>>> >>>>>> >>>>>> >>>>>>> </api> >>>>>>> >>>>>>> wdyt? >>>>>>> >>>>>>> >>>>>>> http://codereview.appspot.com/4171051/ >>>>>>> >>>>>> >>>>>> Im putting a prototype covering these different cases. >>>>>> >>>>> >>>>> >>>> >>> >> >
Sign in to reply to this message.
On Thu, Feb 17, 2011 at 12:54 PM, Michael Hermanto <mhermanto@gmail.com>wrote: > > > On Wed, Feb 16, 2011 at 1:32 PM, John Hjelmstad <fargo@google.com> wrote: > >> On Wed, Feb 16, 2011 at 1:30 PM, Michael Hermanto <mhermanto@gmail.com>wrote: >> >>> >>> >>> On Wed, Feb 16, 2011 at 1:18 PM, John Hjelmstad <fargo@google.com>wrote: >>> >>>> On Wed, Feb 16, 2011 at 11:32 AM, Michael Hermanto <mhermanto@gmail.com >>>> > wrote: >>>> >>>>> >>>>> >>>>> On Tue, Feb 15, 2011 at 3:20 PM, John Hjelmstad <fargo@google.com>wrote: >>>>> >>>>>> Michael - >>>>>> >>>>>> Great to get this going. Comments inline. >>>>>> >>>>>> On Mon, Feb 14, 2011 at 6:21 PM, Michael Hermanto < >>>>>> mhermanto@gmail.com> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Mon, Feb 14, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: >>>>>>> >>>>>>>> Looong comment, partially devised from thinking about this for a >>>>>>>> while... >>>>>>>> >>>>>>> >>>>>>> Thanks for starting this thoughtful discussion. >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>>>>> File features/src/main/javascript/features/rpc/rpc.js (right): >>>>>>>> >>>>>>>> >>>>>>>> http://codereview.appspot.com/4171051/diff/3002/features/src/main/javascript/... >>>>>>>> features/src/main/javascript/features/rpc/rpc.js:721: >>>>>>>> gadgets.rpc.config >>>>>>>> = function(config) { >>>>>>>> As I've been thinking about this some more, I'm starting to think >>>>>>>> that >>>>>>>> our best solution is going to be an improved export mechanism. >>>>>>>> >>>>>>>> It helps me to state quite simply what the goals are. >>>>>>>> 1. Enable property obfuscation. >>>>>>>> 2. Enable exports-as-imports. >>>>>>>> >>>>>>> >>>>>>> Please elaborate this one. >>>>>>> >>>>>> >>>>>> Aka "exports-as-externs," the ability to generate an "externs" symbol >>>>>> file (for compilation) from a given JS bundle without doing so manually. For >>>>>> the vast majority of cases this won't even be needed though, since JS using >>>>>> these libs will either be included in the bundle (so the props will compile >>>>>> appropriately) or be written in some other way, such as inlined JS in HTML >>>>>> using gadgets.rpc.call(..) >>>>>> >>>>> >>>>> Is this merely just dumping all the content of <exports type=js> >>>>> directives to a flat file ? I agree that we should only specify this in one >>>>> place (feature.xml for features makes sense), but it'll also introduce >>>>> another "way" to specify externs (something that people need to learn >>>>> again). >>>>> >>>> >>>> Yes and no, to some extent. This is really only useful for external >>>> callers, such as users of this JS within Google who also use Closure >>>> Compiler. >>>> >>>> As you note, feature.xml is where exports will be. What I'd like is to >>>> avoid these users manually writing an externs.js file, and instead referring >>>> to one that's automatically-generated from the feature itself. >>>> >>> >>> Right, we may also need to support param/return annotations to the >>> resulting externs file, for type-checking purpose when the externs file is >>> used to the another compiler. >>> >> >> Yep, I think Closure Compiler actually does this (?). >> >> >>> >>> >>>>>> 3. Enable export of only "requested" symbol features to uncompiled >>>>>>>> users. >>>>>>> >>>>>>> >>>>>>> >>>>>>>> As written here, I'm not sure how we'd export gadgets.rpc, either >>>>>>>> using >>>>>>>> the "current" method (window["gadgets"]["rpc"] = gadgets.rpc) or any >>>>>>>> other, since we're hiding the gadgets.rpc decl within an anonymous >>>>>>>> closure. >>>>>>>> >>>>>>>> Interestingly, in the runtime-compiled case the existing, unmodified >>>>>>>> mechanism achieves property compression in the ideal fashion: users >>>>>>>> of >>>>>>>> gadgets.rpc get properly translated >>>>>>>> >>>>>>>> eg. library dynamic-height uses gadgets.rpc.call(...). rpc.js will >>>>>>>> get >>>>>>>> obfuscated to something like: >>>>>>>> g.P = (function() { // "gadgets.rpc = ..." >>>>>>>> stuff >>>>>>>> return { >>>>>>>> Q: function() { /* call(...) method */ }, >>>>>>>> R: function() { /* setupReceiver(...) method */ }, >>>>>>>> }; >>>>>>>> })(); >>>>>>>> >>>>>>>> dynamic-height, being compiled in the same unit, will have: >>>>>>>> gadgets.rpc.call(...) >>>>>>>> ...changed to: >>>>>>>> g.P.Q(...) >>>>>>>> >>>>>>>> ...which works. >>>>>>>> >>>>>>>> >>>>>>> Correct this works. >>>>>>> >>>>>>> But, currently, anything (say a new function X) return {}'ed from >>>>>>> gadgets.rpc (or g.P) will always be part of the compiled code, ie: even if >>>>>>> 1) not referenced from global/window, 2) not part of "requested" symbols, 3) >>>>>>> never used by client code. However, it should be removed for >>>>>>> EXPLICIT_RUN_TIME. >>>>>>> >>>>>> >>>>>> That's true for anything returned like: >>>>>> return { >>>>>> 'symbol': foo, >>>>>> 'another': bar >>>>>> }; >>>>>> >>>>>> I'm actually now agreeing w/ you that property compaction is a very >>>>>> good idea, so we should not quote these. As discussed above, we can turn on >>>>>> property compaction and all JS in the compiled bundle will work >>>>>> appropriately. @see next comment for more. >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>>> The main challenge is when someone wants access to g.P.Q by calling >>>>>>>> gadgets.rpc.call. For this, we need a smarter export method. I >>>>>>>> wonder if >>>>>>>> something like this would work: >>>>>>>> >>>>>>> >>>>>>> I don't follow here, the above example is someone calling >>>>>>> gadgets.rpc.call() and it translates fine to g.P.Q, no? >>>>>>> >>>>>> >>>>>> Yes. I should have stated more clearly -- the main challenge is when >>>>>> an "external" caller (not in the JS bundle) wants to reference g.P.Q by its >>>>>> exported name, gadgets.rpc.call. >>>>>> >>>>>> >>>>>>> >>>>>>> jsExport(obj, namespace, propMap); >>>>>>>> >>>>>>>> For gadgets.rpc, we'd generate: >>>>>>>> >>>>>>>> jsExport(gadgets.rpc, "gadgets.rpc", { call: "call", setupReceiver: >>>>>>>> "setupReceiver", ... }); >>>>>>>> >>>>>>>> When compiled, this turns into: >>>>>>>> yZ(g.P, { Q: "call", R: "setupReceiver" }); >>>>>>>> >>>>>>>> The jsExport method would then: >>>>>>>> 1) if propMap && typeof obj === "object", >>>>>>>> A) export window["gadgets"]["rpc"] = g.P --> abstractly, export the >>>>>>>> given symbol on window. >>>>>>>> B) export properties directly by name, as singletons. Iterate >>>>>>>> through >>>>>>>> propMap, exporting properties by name, eg. >>>>>>>> window["gadgets"]["rpc"]["call"] = g.P["Q"]; >>>>>>>> >>>>>>> >>>>>>> Who's generating these jsExport(...) statements ? The JS serving >>>>>>> mechanism based on current <exports> or JS developer ? >>>>>>> >>>>>> >>>>>> JS serving mechanism. The difference is that instead of exporting >>>>>> directly on window, it delegates to a "smart" JS function to do so. >>>>>> >>>>>> So if your <exports> are: >>>>>> gadgets.flash.embedFlash >>>>>> gadgets.rpc.call >>>>>> gadgets.rpc.setupReceiver >>>>>> >>>>>> ...your generated exports are currently something like this: >>>>>> window["gadgets"]=gadgets; >>>>>> window["gadgets"]["rpc"]=gadgets.rpc; >>>>>> window["gadgets"]["rpc"]["call"]=gadgets.rpc.call; >>>>>> etc etc >>>>>> >>>>>> With the help of a smart export function, we can make this both richer >>>>>> (supporting exported closured-Objects) and more compact. The minimal would >>>>>> be: >>>>>> jsExport("gadgets.flash", [ gadgets, gadgets.flash, gadgets.flash ], { >>>>>> embedFlash: "embedFlash" }); >>>>>> jsExport("gadgets.rpc", [ gadgets, gadgets.rpc ], { call: "call", >>>>>> setupReceiver: "setupReceiver" }); >>>>>> >>>>>> The implementation for jsExport: >>>>>> >>>>>> function jsExport(namespace, components, opt_props) { >>>>>> var nsParts = namespace.split("."); >>>>>> var base = window; >>>>>> var prevBase = null; >>>>>> var part = null; >>>>>> for (var i=0; part=nsParts.shift();) { >>>>>> // export "namespaces" >>>>>> base[part]=components[i++]; >>>>>> prevBase = base; >>>>>> base=base[part]; >>>>>> } >>>>>> var props = opt_props || {}; >>>>>> var exportProps = function(root) { >>>>>> for (var prop in props) { if (props.hasOwnProperty(prop)) { >>>>>> root[props[prop]] = root[prop]; >>>>>> } } >>>>>> }; >>>>>> if (typeof base === "object") { >>>>>> // "singleton" object ie. gadgets.rpc w/ props "call", >>>>>> "setupReceiver" etc. >>>>>> // just export the symbols immediately >>>>>> exportProps(root); >>>>>> } else if (typeof base === "function") { >>>>>> // "closured" object such as shindig.uri with methods "getQuery", >>>>>> "setFragment" etc. >>>>>> // create an wrapper to this function that exports any resulting >>>>>> properties it returns in an Object >>>>>> var exportedFn = function() { >>>>>> var result = base.apply(null, arguments); >>>>>> if (typeof result === "object") { >>>>>> exportProps(result); >>>>>> } >>>>>> return result; >>>>>> }; >>>>>> prevBase[part] = exportedFn; >>>>>> } >>>>>> } >>>>>> >>>>>> >>>>> >>>>> Thanks for making this easy. I'm trying this out now in >>>>> AbstractJsCompiler. >>>>> >>>> >>>> Warning! I wrote this in e-mail, so I guarantee it's not quite right. >>>> I'll translate it to a Shindig CL so we can discuss more precisely. We'll >>>> merge our work thereafter. >>>> >>> >>> As talked offline, I'm already on this. >>> >> >> Will not duplicate effort :) >> > > Uploaded new patch. Tested to work across all export cases (as Javadoc'ed), > including prototype'd properties. > PTAL. > The bulk of this work is in http://codereview.appspot.com/4175057/. > > >> >>> >>>> >>>>> >>>>> >>>>>> >>>>>>> >>>>>>>> 2) else if propMap && typeof obj === "function", assume to be a >>>>>>>> Closured-style object constructor. >>>>>>>> A) do same as #1, only export window["foo"]["bar"] as a function >>>>>>>> that >>>>>>>> in turn exports the properties it has been given, by name. >>>>>>>> >>>>>>> >>>>>>> If I understand this correctly, you want to allow 2 styles to have >>>>>>> symbols exported. Does one offer a benefit that the other doesn't? I >>>>>>> personally would go after 1 style, that works. Refactoring all to either >>>>>>> style is easy. And I personally think the object-style declaration is more >>>>>>> obvious -- symbols don't get mapped by some JS execution/helper, less >>>>>>> indirections (ie: no return statements). Thoughts? >>>>>>> >>>>>> >>>>>> AFAIK there will always be >1 way to write exported JS since you can >>>>>> have singletons and you can have constructed "Objects" with methods on them. >>>>>> Exporting singletons is pretty easy in any case. Exporting methods is harder >>>>>> when you use property compression. >>>>>> >>>>>> That's true whether you use Closured or prototyped objects, since >>>>>> methods on each are properties. One advantage of the above mechanism is that >>>>>> it works equally well for both styles of object. >>>>>> >>>>>> >>>>>>> >>>>>>> How about internal objects like rpc.[a|c|t ...], do those need to be >>>>>>> quoted still? >>>>>>> >>>>>> >>>>>> With an export method,* nothing* needs to be quoted, now that I think >>>>>> of it, unless that "thing" is an API that's used by non-compiled contexts. >>>>>> >>>>>> So: >>>>>> 1) internal property keys need not be quoted, since they're only >>>>>> referenced internally and thus obfuscated consistently >>>>>> >>>>> >>>>> Yes, also consistently across features (well, because the compiler >>>>> knows all the features at once). >>>>> >>>>> >>>>>> 2) parameter-bag property keys *do* need to be quoted. Toy example: >>>>>> suppose we had library Point that has method setCoords(params) which takes { >>>>>> x: foo, y: foo }. 'x' and 'y' need to be quoted since external callers will >>>>>> pass in param bags with these as keys. >>>>>> >>>>> >>>>> Yes. >>>>> >>>>> >>>>>> 3) rpc.foo is an interesting case. Technically they do not need to be >>>>>> quoted, since in any situation both sides of gadgets.rpc communication >>>>>> should be using the same bundle and thus consistent obfuscations. However, >>>>>> we should probably quote them anyway for consistent debugging, and because >>>>>> bundles might be asymmetric yet complementary (one side has "rpc" while the >>>>>> other has "rpc:dynamic-height" leading to different obfuscations). >>>>>> >>>>> >>>>> Yes, we've talked about this, rpc.[t|c|...] are used publicly by client >>>>> (ie: rpc.f to find source caller). I don't think you intend clients to use >>>>> this in the first place, because otherwise, I'd come up with more meaningful >>>>> names. >>>>> >>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>>> This whole edifice would allow us to: >>>>>>>> 1. Use property compression optimally. >>>>>>>> >>>>>>> >>>>>>> Yes. >>>>>>> >>>>>>> >>>>>>>> 2. Export only "some" symbols externally. >>>>>>>> >>>>>>> >>>>>>> Yes. Currently, "some" is determined by symbols exported in >>>>>>> explicitly-listed features. >>>>>>> >>>>>>> >>>>>>>> 3. Export singletons, Closured, and prototyped objects the same way: >>>>>>>> <api> >>>>>>>> <exports type="js">foo.bar.method1</exports> >>>>>>>> <exports type="js">foo.bar.method2</exports> >>>>>>>> ... >>>>>>>> >>>>>>> How do these export prototyped objects? >>>>>>> >>>>>> >>>>>> @see above for current thought on syntax. Everything up to the last >>>>>> "." becomes the "root" namespace; all the properties for the same foo.bar >>>>>> prefix become the propMap. >>>>>> >>>>>> wdyt? >>>>>> >>>>>> >>>>> One thought ... can we assume a feature can only export namespace (ie: >>>>> everything before the last property) that is common across all, as long as >>>>> not on window. This assumption appears to make the code simpler. ie: >>>>> >>>> >>>> I suppose we could impose that a feature export only a single namespace, >>>> as a convention. I don't think it's technically necessary though. Regardless >>>> what happens, a JS request could come in for "rpc:core.util" and the exports >>>> would be generated as if for a single feature that does as you describe >>>> below. >>>> >>>> Interesting idea about the globals feature exporting directly on window >>>> w/ "leaf" symbols "gadgets" et al. >>>> >>> What about the case where we don't want to generate exports for anything >>>> except requested features though? >>>> >>> >>>> Consider a request for "rpc". In this case, the exports for "globals" >>>> wouldn't apply. We'd only export for "rpc", which means we'd have to >>>> generate an exports for "gadgets" and "gadgets.rpc" as well, unfortunately. >>>> >>> >>> Hm, this is tricky. I see two types of dependencies here -- 1) deps by >>> namespace (what you described), 2) deps by functionalities (in feature.xml >>> <dependency> sense). For 1), I'm incline to say to not inline exportJs() >>> statements for globals, but exportJs will be smart enough to make "gadgets" >>> namespace available if not exist. >>> >> >> Agree. >> >> >>> >>> >>>> Cheers, >>>> John >>>> >>>> >>>>> >>>>> A feature cannot do -- >>>>> <exports type=js>gadgets.util.makeClosure</exports> >>>>> <exports type=js>gadgets.rpc.call</exports> >>>>> // unless this translate to 2 jsExport statements, each for >>>>> gadgets.util and gadgets.rpc. >>>>> // I can't think of a non-global feature that does this. >>>>> >>>>> But a feature (like globals) can do -- >>>>> <exports type=js>gadgets</exports> >>>>> <exports type=js>shindig</exports> >>>>> // translate to jsExport(null/window, [], { gadgets : "gadgets", >>>>> shindig : "shindig }); >>>>> >>>>> --j >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>>> </api> >>>>>>>> >>>>>>>> wdyt? >>>>>>>> >>>>>>>> >>>>>>>> http://codereview.appspot.com/4171051/ >>>>>>>> >>>>>>> >>>>>>> Im putting a prototype covering these different cases. >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> >
Sign in to reply to this message.
LGTM http://codereview.appspot.com/4171051/diff/4005/features/src/main/javascript/... File features/src/main/javascript/features/rpc/flash.transport.js (right): http://codereview.appspot.com/4171051/diff/4005/features/src/main/javascript/... features/src/main/javascript/features/rpc/flash.transport.js:68: relayHandle.setup(shake['id'], shake['role']); since this is an internal impl detail, we can get away w/o quoting.
Sign in to reply to this message.
On Tue, Feb 22, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: > LGTM > > > > http://codereview.appspot.com/4171051/diff/4005/features/src/main/javascript/... > File features/src/main/javascript/features/rpc/flash.transport.js > (right): > > > http://codereview.appspot.com/4171051/diff/4005/features/src/main/javascript/... > features/src/main/javascript/features/rpc/flash.transport.js:68: > relayHandle.setup(shake['id'], shake['role']); > since this is an internal impl detail, we can get away w/o quoting. > > Ok, unquoted. Same with callee. > http://codereview.appspot.com/4171051/ >
Sign in to reply to this message.
Still LGTM, submit away! On Tue, Feb 22, 2011 at 5:15 PM, Michael Hermanto <mhermanto@gmail.com>wrote: > > > On Tue, Feb 22, 2011 at 4:40 PM, <johnfargo@gmail.com> wrote: > >> LGTM >> >> >> >> http://codereview.appspot.com/4171051/diff/4005/features/src/main/javascript/... >> File features/src/main/javascript/features/rpc/flash.transport.js >> (right): >> >> >> http://codereview.appspot.com/4171051/diff/4005/features/src/main/javascript/... >> features/src/main/javascript/features/rpc/flash.transport.js:68: >> relayHandle.setup(shake['id'], shake['role']); >> since this is an internal impl detail, we can get away w/o quoting. >> >> Ok, unquoted. Same with callee. > > >> http://codereview.appspot.com/4171051/ >> > >
Sign in to reply to this message.
|
