node-convict icon indicating copy to clipboard operation
node-convict copied to clipboard

Suggestion: Refactor module into separate files.

Open sr-scott opened this issue 5 years ago • 5 comments

I really love the Convict config module. We use it extensively. One problem that we keep running into on new projects is that we have to write manual mocks a lot for integration tests.

This is not a big problem but one limitation is that we lose the ability to easily load the config defaults in the same way that Convict does.

My proposal is to extract the schema parsing code into a new file that could then be used to write mock modules (for Jest in our case) that could then be shared with the wider community.

The reason I would prefer to be able to use source directly from Convict is to allow the mock modules to be easily maintained and kept up-to-date as Convict is updated.

Opening this issue to see if there is any wider interest for this. If so I would be happy to help with the refactoring efforts :)

sr-scott avatar Feb 22 '19 11:02 sr-scott

Hello,

Convict source code is, in fact, now a bit too big for just one file. Convict code has also become too complicated. Refactoring the code into smaller modules is the best that can happen to Convict.

So I would love to see PR in this direction.

Cheers

madarche avatar Feb 22 '19 13:02 madarche

@madarche Same! I would love to contribute, but it feels like too much hassle right now :'(

ronkorving avatar May 08 '19 09:05 ronkorving

#327 ?

A-312 avatar Dec 07 '19 12:12 A-312

@sr-scott Same! I would love to contribute, but it feels like too much hassle right now :'(

Is not so hard:

  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L12-L69: coerce/format declaration
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L70: parser storage declaration
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L71-L183: validate things
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L184-L202: Javascript Format Type : Object, Array, String, Number...
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L203-L306: normalizeSchema the internal schema parser, will understand/process schema given by developper
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L307-L333: Getter arg/env
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L346-L348: Utils functions
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L349-L360: Value getter
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L360-L376: traverseSchema -> get property depending of path given
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L377-L411: format/coerce function to validate properties
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L412-L440: Utils functions : load file + walk
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L444-L687: Convict class (I mean when you did config = convict(schema)
  • line https://github.com/mozilla/node-convict/blob/c5f6798ed4edf5249f61d775ebcfd27bab316602/packages/convict/lib/convict.js#L688-L737: addFormat + addParser functions

A-312 avatar Apr 11 '20 06:04 A-312

@A-312 Are you saying "what the three of you see as a problem to help collaborate is not a problem, and you're wrong"? If so, I think you're missing the point.

Tbh, it's been a year, so I would need to re-evaluate my stance here as I'm sure convict has evolved since then. I can't claim what I thought then is still the case today unless I reinvestigate (which I may consider later).

ronkorving avatar Apr 13 '20 01:04 ronkorving