Common Opal & MRI files should be segregated

[marcandre]

There are files in lib/ that are meant both for Ruby MRI and Opal, in particular lib/parser. There are other files that are meant only for Ruby (e.g. lib/sprockets). I'd strongly recommend splitting the lib folder in two, say common and lib or similar.

This would help avoiding other issues, like the fact that some methods are included in opal-parser when they shouldn't, e.g. uglify. When playing around for #455 I thought I was going crazy when the following:

def uglify(str)
  `which 'uglifyjs'`
  # ...

would make rake dist produce an empty opal-parser file... It didn't cross my mind that this would also be interpreted as literal (and invalid) JS!

I'll make a pull request if you like, just confirm this all makes sense and I'm not missing something.

[meh]

Sincerely, I think it's fine as is, opal-parser is kind of an hack in the first place.

Having unrelated code in the parser is a bug in itself.

[elia]

The uglify stuff is in the builder, and that makes sense to me, anyway it clearly doesn't belong to opal-parser.js.
I'll move them from Opal::Builder::Util to Opal::Util within lib/opal/util.rb.

[marcandre]

We all agree that the Util code needed to be moved.

Still, I'm curious as to what is the perceived advantage of having MRI-only and MRI+Opal in the same directory? It's probably clear for you guys which is which (and even then, someone made the mistake of putting the uglify method in the wrong spot), but for someone new to the codebase like I am, it isn't!

That you follow my recommendation or not, the README should be modified, either to state that common/ contains common or, or else to rectify the current blurb that states, incorrectly:

lib/ code runs inside your ruby env. It compiles ruby to javascript

[meh]

lib/ code runs inside your ruby env. It compiles ruby to javascript

That is true, it's code that runs in your Ruby environment (in this case Opal) and it compiles Ruby to Javascript :P

Anyway opal-parser is generally an exception to that rule, and I agree it should be clarified in the documentation.

[marcandre]

Anyway opal-parser is generally an exception to that rule, and I agree it should be clarified in the documentation.

Doesn't opal-parser require the vast majority of lib/? TBH, I'm not 100% sure which files are opal compatible in there.

Anyways, I'll shut up now and let you guys decide. Sorry if I'm a bit of a newb 😃

[meh]

Yes, most of it, the uglify/gzipping stuff shouldn't have been there in the first place IMHO.