Preparing convict 6

[A-312]

Depending of #350 I will merge all my PRs into my branch to save the time on several rebase/merge/conflict resolving.

TODO :

• rebase into upstream/master
• merge fix: Update Precendence order documentation according implementation #314 (fix Precendence order #211 + fix: Readme precedence lists load as being equivalent to set #208)
• merge fix: Fire error with 'properties' keyname in schema and 'properties' of convict become '_cvtProperties'  #336 (fix: Unable to use the name "properties" within a schema #318)
• merge fix: Add doc about second argument of convict #348
• merge fix: Use Chai for test cases #347 (fix: Some test don't provide error message (they can fail for other thing) #346 + fix: Replace should (=> Last publish 3 years ago) by Chai #345)
• merge fix: Coerce from strings to their proper types (NOT MORE) + code review on coercing #352 (fix: coerce send too many (useless) argument  #338)

---

• pick : baf738a
• pick : 7c667f7
• pick : 8b571e4
• pick : c4f7ae4
• pick : 0dc4d83

Source: #342 (comment)

• merge fix: Custom Getter #342 (fix: Custom Getter #313, fix: Include env var in validation error message #333, fix: 4.0.1 regression #224 + fix: chore: reverting changes from 217 #261, fix: Plugins #283)
• merge fix: Full support of periods in property name and property path #354 (fix: Use 'ObjectPath' package for dot (period) support #351, fix: Cannot use dot in keys when using Object format #250, fix: Error thrown if Object type default value has a property with a period #271, fix: Fix dot-paths in dynamic objects #315)
• merge fix: Improve parsing behavior and default property usage #357 (fix: Improve schema parsing and default property usage #337, fix: Validate the schema to require defaults #254, fix: Providing a default of undefined prevents format from being checked #278, fix: Improve .defaut() and .reset() errors for users #349, fix: Add option: Properties with default of undefined [do/do not] behave like properties that have not been specified #355, fix: We can not do convict(config.getSchema()) #356)

fix: #288

[A-312]

BREAKCHANGES LIST :

• _cvtProperties is a reserved word instead of properties. Now, properties is supported like property name in your schema (doesn't throw an parsing error anymore).

• coerce method will be always applied:
  - Before this version: coerce was only applied with: .set(), .env: and .arg:.
  - With this version: coerce is also applied with: .load(), .loadFile() and .default:.

  This change mean the value (first argument of coerce method) can be another type of String. We advice you to add type checking in your coerce function to avoid an JS error with undefined value. See: array coerce change from: (v) => v.split(',') to: return (v) => (typeof v == 'string') ? v.split(',') : v;

• agregate hack removed (Coerce from strings to their proper types (NOT MORE) + code review on coercing #352).

• Fix .getSchema/getSchemaString() output.

• Parsing behavior with format: [ typeof this !== 'object' ].

CHANGELOG :

Some of my change are tacit, I will try to add all of my tacit change here :

• Error are more human-readable :
  

  code

  var config = convict({
    something: {
      format: function (val, schema) {
          console.log(arguments);
          if (schema.env && val === '') {
            throw new Error(`env[${schema.env}] is not set !`)
          }

[coveralls]

Coverage remained the same at 100.0% when pulling 51c2483 on A-312:feat-multi-packages-split into 30bcc2e on mozilla:feat-multi-packages-split.

[A-312]

@madarche This PR is ready and the 6.0.0 too. 😀

[julian91]

when can we expect 6.0 to hit the shelf? saving quite some time on refactoring due to the fix with the dot notation object properties in the branch.

[A-312]

(The new dot notation doesn't work well with backslash #354 (comment) )

[A-312]

I will make another PR (for 6.0.0), to support default like property name in config. -> #357

[A-312]

My PR is ready @madarche. I hope that all my change will be ok with convict way.

[A-312]

@vladikoff madarche seems to be inactive this month (The last weeks, I didn't get answer in #350), I think he got lot of things to do in IRL. I made lot of change, I don't know if he will have enough time to watch/review to merge my work. Do you think you can have a look ?

Also, @zaverden do you think you can review my changes in the README ? My english is not perfect and that will help the mozilla team to review/merge my PR.

[zaverden]

I have run these changes through grammarly.com and there are results.

[A-312]

Done, thanks you. Is there any other non-sense ?

[vladikoff]

I'm away next week, will take a look when back

[A-312]

ping @vladikoff

Also I would to know : #350 (comment) If I mark .load and loadFile deprecated, and I replace this two by merge function

Something like this :

convict.merge = function (source) {
  if (typeof source === 'string')
    loadFile(source)
  else
    load(source)
}

[A-312]

@madarche @vladikoff I don't want to be insistent but I made a lot of work to improve convict, do you need help for this repository ? Should I wait more ? Should I make my own fork (but I like and understand the convict way & project) ?

[madarche]

@A-312 your work is really really appreciated. I promise I'll deal with this PR before Saturday.

[madarche]

@A-312 this is a quite a big work altogether now, hence the time that it takes to review it.

[vladikoff]

I am traveling again, however I will be able to post a new version to npm when it is ready. Won't have time to deep dive into the changes for a few weeks.

[madarche]

@A-312 I've not totally finished to read all the proposed changes. But at this point there is one change that I don't agree with: it's the use of Chai.js, because Chai.js has a design mistake: cf. https://github.com/moll/js-must/#asserting-on-property-access. With Should.js it's still possible to use the unsafe syntax, but at least it's not recommended/suggested.

So I'll not merge this specific PR, but will use it, as a guideline, to do the merge work on my side. Thanks for all the work nonetheless.

[A-312]

@madarche It is why I use expect(problem).to.not.exist()

EDIT:

The output of must is hard to read and less readable than chai.js. To many conflict, you will never have enough time to merge all my change...

[A-312]

Can we discuss about that on IRC or discord, or other thing ?

[madarche]

@A-312 I've merged the first PRs and merged the feat-multi-packages-split branch into master.

Please don't work on any other PR or modification for now.

I'll be online in a very asynchronous manner in the following days. But I'll continue the review work.

Thank you

[A-312]

I edited each test... You will have a lot of work to do. I don't understand why you don't want use chai.js because I used expect(), and I don't got any bug.

[A-312]

I will do the thing on my own way. (take the time you want, don't waste your time on this PR if you have lot of things to do IRL, I know it takes a long time to resolve conflicts)

Also i want to stay author of my commit/change i made.