Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Getter #342

Closed
wants to merge 26 commits into from
Closed

Custom Getter #342

wants to merge 26 commits into from

Conversation

A-312
Copy link
Contributor

@A-312 A-312 commented Dec 9, 2019

fix: #313

example of custom getter :

convict.addGetter('default', (value, schema, stopPropagation) => schema._cvtCoerce(value));
getters.order.push('value');
convict.addGetter('env', function(value, schema, stopPropagation) {
  return schema._cvtCoerce(this.getEnv()[value]);
});
convict.addGetter('arg', function(value, schema, stopPropagation) {
  const argv = parseArgs(this.getArgs(), {
    configuration: {
      'dot-notation': false
    }
  });
  return schema._cvtCoerce(argv[value]);
}, true);

TODO :

NB :
fix: #224 => with stopPropagation() in a customGetter. & fix: #261 :

convict.addGetter('env', function(value, schema, stopPropagation) {
  if (typeof this.getEnv()[value] !== "undefined") {
    stopPropagation();
  }
  return schema._cvtCoerce(this.getEnv()[value]);
}, true);

fix: #283

@coveralls
Copy link

coveralls commented Dec 9, 2019

Coverage Status

Coverage decreased (-0.2%) to 99.805% when pulling 3ad8b5f on A-312:custom-getter into d263763 on mozilla:master.

@A-312
Copy link
Contributor Author

A-312 commented Dec 22, 2019

I merged #347 in this one because some test don't work well. Some test don't check the throw message corresponding to the expected error. And without expected/actual in the errorlog, it's hard to understand the problem.

with current NPM registry state
@A-312 A-312 mentioned this pull request Dec 28, 2019
@A-312
Copy link
Contributor Author

A-312 commented Dec 30, 2019

conf.sortGetters() is different of convict.sortGetters() (where conf = convict(schema)). The first will change only local config, and the second the global config but we have to do conf.refreshGetters() to apply global change in local config. We are able to set different getter order for each conf we have.

@A-312 A-312 marked this pull request as ready for review January 1, 2020 00:45
@A-312
Copy link
Contributor Author

A-312 commented Jan 1, 2020

Ready :D

EDIT :
but based into mozilla:master branch 😕 I will rebase into 6.0.0 branch.

@A-312
Copy link
Contributor Author

A-312 commented Jan 1, 2020

For the rebase into 6.0.0 branch. I think, i will make another PR and pick these commits in:

I will wait that #336, #347 and #352 is merged before rebase.

@A-312 A-312 mentioned this pull request Jan 5, 2020
14 tasks
…yValues()` name is similar that `applyGetters()`, is more understandable
@A-312
Copy link
Contributor Author

A-312 commented Jan 5, 2020

Rebased into #353

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4.0.1 regression Custom Getter Plugins
4 participants