Build system introduces redundant boilerplate

[dead-claudia]

I noticed that the build system has created a lot of boilerplate and indirection in the bundle. It's effectively a concatenation script, but with some special semantics that are plainly very unidiomatic, and introduce numerous otherwise-unnecessary closures at load time.

To give an example, here's a snippet from your src/[email protected]

this.err = function(errorType, errParams) {
  errParams = this.normalize(errParams);
  return this.errorListener.onErr(errorType, errParams);
};

You could just as easily do this instead:

Parser.prototype.err = function(errorType, errParams) {
  errParams = this.normalize(errParams);
  return this.errorListener.onErr(errorType, errParams);
};

Doing this would let you just make your build script a glorified cat program, that just happens to wrap everything in an IIFE.

This would be much simpler to write and maintain, even if you decided to keep the naming convention. In fact, UglifyJS2 already does similar.

[FailorTwist]

@isiahmeadows
Yes.
I have to agree the build system is the ugliest part of this parser; the only reason that made me adapt that unusual way was that I wanted to keep the modules (i.e, <module-name>@<class-name>.js) isolated from each other, and from the messy top-level scope with all the constants, utility functions, etc.

The only way that came to my mind to satisfy this constraint was wrapping each module in an IIFE, calling every given IIFE with this set to the prototype of the module's owner class at parser's load time.

This enabled me to declare whatever module-private thing I found fit, but in retrospect, I have to admit this approach has not found much use in the actual codebase, even though it has saved me from getting caught by couple of bugs in more than one occasion.

@zenekron actually suggested a while back that we should adapt a more modular approach in the codebase, but back then my reasoning was that, since the submodules are not technically reusable components, concatenating them will do; furthermore, one of my troubles was modularizing files that contained constants -- it turned out to be very error prone in the fragile context that parsing happened in; for example, considering we have a constant named PREC_MULTIPLY,

var PREC = require('src/precedence-constants.js');
...
// please note the typo -- but it will go on even though it shouldn't
parser.parseNonSeq(null, PREC.MULITPLY);
...

versus

...
// this one will fail as an immediate reference error -- PREC_MULITPLY is not defined (PREC_MULTIPLY is)
parser.parseNonSeq(null, PREC_MULITPLY);
...

I also tried linters back then, but the amount of false positives it gave me due to the unusual decisions I had to make in the performance-critical parts of the code actually decreased my productivity.

All that said, the build system is terrible even though not broken yet; I wish I had the time (and the expertise, and the Internet, since github is inaccesible in my country most of the time) to make a better one (my day job is a 7to9 one totally unrelated to programming, and it leaves me too spent to find time for making things better, except maybe at the weekends.)

P.S(Ssssssssst!) -- I took a very quick look at the custom builder you have written for your website, and it was plain amazing; 😎

[dead-claudia]

@FailorTwist Check this out.

P.S(Ssssssssst!) -- I took a very quick look at the custom builder you have written for your website, and it was plain amazing; 😎

Thanks. But actually, most of the build logic is interfacing with other tools (I wrote it in Pug and Stylus, and the blog is a mixture of Markdown, YAML, and JS, but I generate an Atom feed for it). And I can tell you the blog half is a massive, brittle hack - I can't even delete old posts until I fix part of it, and it doesn't yet support pagination. I can also tell you I had to create a pseudo-parser to handle generating the previews. But at least going down the wire, it's a featherweight.

But yeah, I'm not exactly proud of the blog handling, like I am of the concurrent task runner.

[FailorTwist]

@isiahmeadows

it's a featherweight.

Lean, I would say, and that was my exact impression when I first came across it; I wish this parser could also have a builder like that.

@FailorTwist Check this out.

@zenekron actually recommended it recently but I had nearly zero time to seriously consider it back then.

just a quick question about it, as I can access neither rollupjs.com, nor npm, and I can't clone it/download its zip from github to test it by myself (I can only browse its repo) -- considering it uses import/export constructs to manage modules, does it mean the bundling process has a transpilation step in-between?

[dead-claudia]

@FailorTwist

Lean, I would say, and that was my exact impression when I first came across it; I wish this parser could also have a builder like that.

In all honesty, it comes down to keeping things simple. Hence why I recommended killing 99% of the build system. The less that gets emitted, the better.

does it mean the bundling process has a transpilation step in-between?

Nope. It works at the syntax level directly, linking them as bindings rather than values, per the spec. In addition, it does limited dead code elimination with the exported bindings. That's how it can create bundles so small.

[icefapper]

Nope. It works at the syntax level directly

Thanks for the response; that is great news!
I have been bitten more than once by transpilers (they are human devices, after all, and in two occasions , back when I had started working on this parser, I came across weird bugs that turned out to be due to faulty transpilation.)
This makes me have an obsession about not using transpilers when brittle/delicate/fragile-logic code is involved. A largely unfounded obesession, I concede, but I need time to recover from those terrible experiences.
But the fact that rollup is basically ES3/ES5-syntax+import/export makes it a very considerable choice.

I have already refactored a fair amount of code in the baseline branch; as I have no experience with rollup, could I ask you take a look and estimate how much it might take to migrate to rollup, and what changes should be made to the codebase to make it ready for the migration?

[dead-claudia]

Probably shouldn't take much beyond what it'd take to migrate to a concatenation script - maybe a few more exports.

[icefapper]

@isiahmeadows
Thank you; I will start working on it as soon as I get a couple of other issues fixed on my local repo.

[dead-claudia]

I'll note one major thing: Rollup requires a plugin (rollup-plugin-commonjs) in order to interact with npm stuff, but you'll only need it if you use any CommonJS modules (i.e. most modules on npm).

Oh, and a couple years from now, after Node and browsers finally implement ES modules natively, you won't need Rollup, either.