Hi Josh, Although I prefer one file with all info (from a user perspective seems more easy since user don't use to know the internals of the technology and why things finally are in a form or another), for me is ok to change it if that will be faster and better.
Maybe the best way to know if is a better way is just trying it. If you finally see it works better, I think the decision is easy :) Thanks El mar., 20 ago. 2019 a las 20:29, Josh Tynjala (<[email protected]>) escribió: > > but opening almost every JS file twice doesn't make sense to me. > > We agree completely on this. > > -- > Josh Tynjala > Bowler Hat LLC <https://bowlerhat.dev> > > > On Tue, Aug 20, 2019 at 11:21 AM Alex Harui <[email protected]> > wrote: > > > Doesn't your plan assume that searching for a potentially (and probably) > > missing companion file is faster than opening and scanning a few lines of > > an known-to-exist file? That's opposite of what I thought was the case. > > > > I haven't looked closely at how things work today, but I thought we were > > searching every JS file for @extern annotations (as well as > remove-circular > > info) and adding files with @externs to the externs list. Maybe that > code > > no longer runs these days with recent changes to better handle source > > externs. But also check externs from typedefs SWCs. There may be other > > ways files get added to the externs list. > > > > I'm not opposed to refactoring to improve performance and consider > > separating dependency on the Closure Compiler, but opening almost every > JS > > file twice doesn't make sense to me. > > > > Another idea may be searching the externs as they are being copied to the > > output folder. Maybe there should be some data structure that various > file > > handlers contribute to. Maybe there should be more Publisher classes > with > > a base class that the Closure Publisher overrides to further scan the > files > > for remove circular dependencies. > > > > Maybe I'm too old, but my recollection was that opening files was more > > expensive than reading opened files so I'm hoping we can avoid doing > that. > > > > -Alex > > > > On 8/20/19, 11:03 AM, "Josh Tynjala" <[email protected]> wrote: > > > > Since externs can also include inject_html, and externs don't have > > circulars to remove, it would faster to find inject_html in those > files > > using this proposed method. > > > > It's true that remove-circulars already opens and reads the JS files. > > However, by including the check for inject_html in there, we had code > > that > > was more complicated than it needed to be only to gain the > > optimization of > > opening and reading JS file a single time. Extracting the inject_html > > detection makes the GoogDepsWriter code more focused, which means > it's > > less > > effort for a developer to understand what the code is doing. In my > > experience, there's a lot to process there to fully understand it > all. > > One > > fewer thing will help. Additionally, in the future, should we decide > > to add > > a separate workflow that uses something other than Closure compiler > to > > create a release build (maybe something involving the newer native JS > > modules), the inject_html detection code will not be as tightly > > coupled and > > can be reused more easily. > > > > Finally, in my opinion, the less we need to read generated JS files > to > > search for special strings, the better. It may not be possible to > > avoid it > > completely, but this is a place where we can avoid that. > > > > -- > > Josh Tynjala > > Bowler Hat LLC < > > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7Ce071bbe36e6848e8731208d72598c074%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637019210332973330&sdata=0cNEXnLCkz48JYr3RRXgyJj%2FMQzc%2Fp27pCT70ozU8DQ%3D&reserved=0 > > > > > > > > > On Tue, Aug 20, 2019 at 10:31 AM Alex Harui <[email protected] > > > > wrote: > > > > > IMO, we have to open and read each JS file anyway for > > remove-circulars, so > > > it isn't clear that looking for additional companion files is going > > to be > > > faster than scanning an already open file. We could make the > > inject_html > > > easier to find by moving it higher up in the JS file. > > > > > > My 2 cents, > > > -Alex > > > > > > On 8/20/19, 10:25 AM, "Josh Tynjala" <[email protected]> > > wrote: > > > > > > I would like to propose a change to how we handle inject_html. > > > > > > Currently, it works like this: We define an <inject_html> tag > in > > an > > > asdoc > > > comment. That tag gets copied to a jsdoc comment when we emit > JS > > code. > > > When > > > we build an app, we search through the JS files line by line to > > find > > > <inject_html>. > > > > > > I think that, when we emit JS files, it would be beneficial to > > emit a > > > separate file that contains the inject_html content. > > > > > > So, you'd get a JS file, like we have now: > > > > > > com/example/MyClass.js > > > > > > Then, if the class contains inject_html, you'd also get: > > > > > > com/example/MyClass.js.html > > > > > > This is similar, in a way, to how there's a MyClass.js.map file > > when > > > source > > > maps are enabled. > > > > > > With this change, instead of loading MyClass.js and parsing it > > line by > > > line > > > looking for inject_html, we could simply check to see if > > > MyClass.js.html > > > exists in the same folder (or in the same SWC). This should > help > > us > > > detect > > > inject_html much faster and without needing to try to parse JS > > files. > > > > > > Thoughts? > > > > > > -- > > > Josh Tynjala > > > Bowler Hat LLC < > > > > > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7Ce071bbe36e6848e8731208d72598c074%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637019210332973330&sdata=0cNEXnLCkz48JYr3RRXgyJj%2FMQzc%2Fp27pCT70ozU8DQ%3D&reserved=0 > > > > > > > > > > > > > > > > > > > > -- Carlos Rovira http://about.me/carlosrovira
