About 0.6-dev

[zenekron]

@icefapper I noticed the effort you put into splitting the codebase into smaller components but unfortunately, in my opinion, it's not completely on the mark yet.

For now you should only think about node's environment and focus on making the best use of its module system. A practical example of this would be what I've done in the parser folder where:

1. Prototype functions are defined one-per-file in parser/parse and parser/util.
2. Shorthands that are meant to be used by only a function (like #asArrowFuncArgList and #asArrowFuncArg being called only by Parser#parseArrow) are now bound to that function module's scope.
3. The constructor is a function defined in parser/constructor.js.
4. The parser's entry point is parser/index.js which automatically binds the prototype to its constructor.

   Pros & Cons

   Pros

   • Hierarchical organization of modules
   • No global space pollution (like with the _class)
   • Better scope management thanks to modules (you expose only what you need)
   • Natively supported by node (no need for compilation/transpilation of any sort)

     Cons

   • The browser version requires to be generated via browserify

     Additional notes

While the use of browserify might look a bit restrictive, it is very advantageous instead. Why? It can generate UMD builds!. This way jsRube would work as intended independently of how it is loaded in the browser (be it a src tag, an AMD module loader a CommonJS module loader or whatever).

By the way, if you want to discuss privately, I'm always avaiable at zenekron@gmail.com.

[icefapper]

Hello Thinking again, I guess I should give up on my strict abstinence from dependencies. I reached this conslusion after viewing 0.6-dev; even though they have not (yet) posed a radical change on the structure, the changes look great. I just wonder why you did not do it directly on the master branch. It deserves to be there.

tl;dr Your code looks much more manageable than mine, and, if it is passing the tests, it deserves to be directly on the master branch.

Thanks again for all your efforts!

[icefapper]

Just one note about chosing the right node vesion to run jsRube on: in all versions of node before 6x, it abrubtly ran into crashes while benchmarking "real-world" cases, like jquery1.4.2-mobile, etc.

Please take this issue into account while changing the node version. Thanks again!

[zenekron]

@icefapper Didn't want to push to the master branch because of my poor git skills and because this is just a proposal (only a few functions have been moved and I've been very rough while doing so).

About the node version I thought the bug had been already fixed, I should've paid more attention there!

[icefapper]

@zenekron I've actually been sold by your proposal and am now refactoring accordingly! Could I ask you how should I adjust the test files (quick-test.js and build-and-test.js)? I have never used browserify and I'm clueless how I can make the ./dist/lube.js now that we are going to depend on browserify :\

[icefapper]

@zenekron also, what is your idea about having an IRC channel for our discussion? I've created a #jsrube channel (on freenode) but I'm not sure how of value it would be to development speed

[zenekron]

@icefapper I will be happy to demonstrate how to build and test the library, unfortunately that can't be done until all the code has been refactored. Anyway I can assure you that it's a pretty simple process that wont require too much effort.

I proposed the mail only because I'm not really used to open issues only to discuss about some features, but it's not a problem for me.

IMO if in the future you feel the need to create some sort of chatroom for the project, you might want to take a look at gitter before creating IRC channels.

[icefapper]

Thanks for mentioning gitter; looks great; I will give it a try! As for the refactoring thing, I've done some; in fact, it's almost done, except that I have no clue how to make modules in src (which are the "global" modules) accessible to those under src/parser/ :\

That would be crucial in order to supply the "actual" parse modules with constants they use.

[zenekron]

@icefapper I'll step in to sort a couple things out :)

[icefapper]

Thanks a lot!

[icefapper]

@zenekron I just created the jsrube gitter "channel"!

[zenekron]

@icefapper The porting has been completed, the modules behave correctly but, as I said in f10a774df68b044fa232c6e59a8e3fa6f053594c there must have been some minor issue during the porting that makes the parser fail.

Let me hear what you think about it.

[icefapper]

@zenekron Wow! Thanks for all the efforts, the code base looks much cleaner now; I just can't believe it!

As for the errors, we had both made a few :wink: Mine was the unforgivable one though; it was the aftermath of a careless replace (vim's :%s/ cough/g) of 'function [something]' with 'module.exports.[something] = function'. I hope I have corrected it.

Yours was a minor one indeed. A lesson I dearly paid for back when I started this project was:

This in an object literal does not refer to the object literal it is contained in.

Also for some real-world cases (notably angular) the parser gets into infinite loops; strange, because that does not happen in the master branch's version of the parser. I have no immediate clues as to where this is happening though, and I'd be glad should you help me browserify it so I can inspect it in the venerable chrome environment :wink:

cleaning and refactoring the mess into something this modular and readable is something I couldn't have done all by myself. Thanks again for the time :)

P.S. refactoring for this particular code base might have a couple minor performance issues to beware of, but that would be something that can be taken care of after we get the tests passing.