Code Readability Suggestions

[nmccready]

First off this project is really cool and I am trying to understand it and contribute to it.

However, I am finding the codebase significantly difficult to read.

This difficulty to me seems to extend from the heavy usage of arrays to be utilized later as function arguments.

Many of the array usage stems from entryPoint andproxyLambdaRequest which in turn get pumped into a zero-process. This process boils down to 8 function arguments. This would be a ton more readable if structuring and destructuring were used instead.

Areas where the code is very difficult to read for me begin in buildManifest.js.

Which has led to comments such as:

  // get all related files (imports/requires) of this lambda
  lambdas = lambdas.map(endpoint => {
    /*
      TRYING TO FIGURE THIS OUT
      endpoint at this moment is:
        [urlPath, relativeFilePath, lambdaType]
      to become:
        [urlPath, relativeFilePath, lambdaType, [relativeFilePath, ...dependencyTreePaths]]
    */
    endpoint.push(
      [endpoint[1]].concat(dependencyTree(endpoint[2], endpoint[1]))
    );
    return endpoint;
  });

Anyway, I don't mean to be critical I just want this to be easy to help out.

[nmccready]

Also see here:

https://stackoverflow.com/questions/12826977/multiple-arguments-vs-options-object#answer-12827204

Also no more than 3 arguments is possible consideration when deciding to switch to options object arg.

https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)

[asadm]

I absolutely agree with this! The project needs a cleanup. As you have found, the whole problem stems from the format of the internal manifest, which is full of arrays. I would get to solving this and the resultant argument hell asap! (currently working on making the latest release stable)

In the meantime, if you feel like you can give it a shot please go ahead!

[nmccready]

Ok, I have a PR coming soon with an additional way to deal with middlewares where most my issues stem from loading a middle ware that support dynamic swagger. Should make more sense in the PR. But I have a basic express example without zero which I would like to work with zero.

https://github.com/nmccready/swagger-express-ts-example

See how to use swagger-express-ts here .

Bottom line is the middleware needs to be loaded prior without a file based route. So I am trying to make this work dynamically by adding a middlewareHandler.

[nmccready]

Another thing you might want to consider is adding lazy logging to your debug usage which would improve performance.

See:

• https://github.com/nmccready/debug-fabulous Benchmarks of many loggers but it does show improvements with lazy logging that I implemented
• https://github.com/nmccready/pino/blob/debug-fab/benchmarks/basic.bench.js#L69-L115

❯ yarn bench-basic
yarn run v1.19.1
$ node benchmarks/utils/runbench basic
Running BASIC benchmark

benchBunyan*10000: 496.290ms
benchBole*10000: 391.516ms
benchDebug*10000: 265.778ms
benchDebugDisabled*10000: 15.453ms
benchDebugFab*10000: 321.426ms
benchDebugFabCb*10000: 211.910ms
benchDebugFabDisabled*10000: 15.509ms
benchDebugFabCbDisabled*10000: 11.169ms
benchLogLevel*10000: 354.771ms
benchPino*10000: 207.691ms
benchPinoExtreme*10000: 83.104ms
benchPinoNodeStream*10000: 165.529ms
benchBunyan*10000: 454.094ms
benchBole*10000: 197.889ms
benchDebug*10000: 233.785ms
benchDebugDisabled*10000: 16.748ms
benchDebugFab*10000: 221.447ms
benchDebugFabCb*10000: 360.479ms
benchDebugFabDisabled*10000: 8.201ms
benchDebugFabCbDisabled*10000: 8.417ms
benchLogLevel*10000: 245.009ms
benchPino*10000: 186.967ms
benchPinoExtreme*10000: 72.358ms
benchPinoNodeStream*10000: 157.134ms

Notice the Disabled levels. Thats with that log level off.

[asadm]

Thanks, I will check the debug module. As for the original issue, #94 and #97 should improve this.

[nmccready]

#94 looks great

Not sure if I should dive into #97 yet for understanding. What's the intent for the emitter to improve ?

[asadm]

Previously, there was a callback hell between file watcher, manifest module and the router. Because on each file change we needed to generate a new manifest and then on a new manifest, we needed to tell the router to update itself (if any pages were added or removed etc).

With emitter model, we just pass these emitters and the module listens for events internally.

Would love you feedback on the updated structure here.