fabrix-app / fabrix

Strongly Typed Modern Web Application Framework for Node.js and Browsers
Other
107 stars 5 forks source link

[Discussion] Eliminate Array from configs including main.spools #30

Closed scott-wyatt closed 5 years ago

scott-wyatt commented 5 years ago

Issue Discussion

Fabrix will attempt to merge an array into it's tuple schema, eg.

main.spools: [
  Spool1,
  Spool2,
  Spool3
]

Becomes:

main.spools.0.Spool1,
main.spools.1.Spool2,
main.spools.2.Spool3

This is very problematic as unsetting or overriding arrays in the config becomes very unpredictable, especially as they are chained and modified by outside spools.
Instead config arrays should be converted to key values where ever possible. eg.

main.spools: {
  spool1: Spool1,
  spool2: Spool2,
  spool3: Spool3,
}

The above is now easily predictable.
This would require a breaking change, so realistically this should go on the v2 branch.

pmould commented 5 years ago

@scott-wyatt, here's a few points i would like to bring up.

In order for config properties to be consistent and to solve the same problem as you mentioned, all the config properties could be objects as well.

Can you give examples of where someone might want to override unset or override themain.spools?

I am not sure on this but is there a significant performance benefit in iterating through Array.forEach instead of using an object using Object.keys to unset or override configs in fabrix? Benchmark mark testing on this would be good to know.

FYI, here's a benchmark that covers Object access and Array access showing that Array access is more effecient: https://stackoverflow.com/questions/17295056/array-vs-object-efficiency-in-javascript

scott-wyatt commented 5 years ago

@pmould Yes, I'd like to get rid of Arrays completely in Fabrix configs, (deprecate them in v2.0, and throw an error in v3.0) and thanks for your feedback!

Fabrix builds configs into "tuples" for example config.get('my.config.value') which is much much faster than doing an object key search or a loop over an array. Each config value is mapped at boot. Set is faster than Array which is why we use tuples, however some benchmarks to prove that it is that much faster in Fabrix would a good idea.

When I rewrote the spool-router, a huge issue was that routes were originally in Hapi style arrays, meaning that trying to override a route from a spool was slow and difficult. This is a really common action, as many spools will chain new values onto a route during their initialize or configure stage in the lifecycle. By switching to tuples in the config, this increased the router's performance by 5-10x, of course that gets slowed down a lot once it get's mapped into Express or Hapi.

As I've been writing more and more Fabrix spools, I've noticed that there was a pattern forming. For arrays, they can't be guaranteed to alway be in order, so we can't reliably do something like config.set('my.config.value.0', 'new value') or config.set('my.config.value', ['new value']). Also, when changing the number of elements on an array, Fabrix's flattener would then need to handle unsetting, resetting, creating key's (that is slow on boot and again unpredictable).

Currently, Fabrix can cold start in under a millisecond, I'd like to get that a lot faster and I think that we could do that by removing arrays from configs. I have yet to find a single config where I am currently using an array, that I couldn't use a key value pair.

scott-wyatt commented 5 years ago

Followup, while writing bobbins I override main.spools frequently to change boot sequences depending on the bobbin. It could also make generating app configs pretty nice as well.

scott-wyatt commented 5 years ago

Will be implemented in 2.0, closing for now.