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

[POSSIBLE BREAKING CHANGES] Create DSL for satpam rule #54

Open
sendyhalim opened this issue Feb 2, 2018 · 2 comments
Open

[POSSIBLE BREAKING CHANGES] Create DSL for satpam rule #54

sendyhalim opened this issue Feb 2, 2018 · 2 comments

Comments

@sendyhalim
Copy link
Member

sendyhalim commented Feb 2, 2018

I think it's better if we have our own DSL for rule e.g.

const satpam = require('satpam');

// With string (current implementation)
const rules = {
  phoneNumber: ['required', 'string'];
  title: ['required', {
    name: 'memberOf'
    params: [['mr', 'mrs']]
  }]
};

// Next implementation could be like this
const rules = {
  phoneNumber: [
    satpam.rules.required(), 
    satpam.rules.string()
  ],
  title: [
    satpam.rules.required(),
    satpam.rules.memberOf({
      acceptedValues: ['mr', 'mrs']
    })
  ]
};

Reasoning behind this is it's easier to read and it's easier to avoid rule typo, because sometimes we do something like this

const rules = {
  phoneNumber: ['require']
};

module.exports = (req, res) => {
  // Uh oh rule typo >.<, it won't happen if we have our own DSL, 
  // would throw error when starting the server
  const  validationResult = satpam.validate(rules, req.body);
  ....
};

Bonus point
we can make it backward compatible, we just need to check old rule format and if it matches then parse the rule using the old implementation

@albertstevelino
Copy link
Member

why using function instead of attributes?
I think satpam.rules.required would be more easier

const rules = {
  phoneNumber: [
    satpam.rules.required, 
    satpam.rules.string
  ],
  title: [
    satpam.rules.required,
    satpam.rules.memberOf('mr', 'mrs')
  ]
};

@sendyhalim
Copy link
Member Author

sendyhalim commented Feb 2, 2018

@astevelinocermati for consistency reasons and some rules have optional parameter(s)

satpam.rules.requied // undefined T_T
satpam.rules.requied() // Will be caught
satpam.rules.required({
  shouldValidate: (input, ruleObject) => input.name === 'ironman' // Will run validation if name is 'ironman'
});

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

No branches or pull requests

2 participants