Allow docs to match examples from #39
Conversation
lib/schemas/FieldSchema.js
Outdated
| 'Defines a field an app either needs as input, or gives as output.', | ||
| description: `Defines a field an app either needs as input, or gives as output. In addition to the requirements below, the following keys are mutually exclusive:\n\n${mutuallyExclusiveFields | ||
| .map(f => `* ${f.map(x => `\`${x}\``).join(' & ')}`) | ||
| .join('\n')}`, |
There was a problem hiding this comment.
This is a little bit hard to read. Can we separate it into a function?
| ['dict', 'list'], // Use only one or the other | ||
| ['dynamic', 'dict'], // dict is ignored | ||
| ['dynamic', 'choices'] // choices are ignored | ||
| ] |
There was a problem hiding this comment.
These exclusive fields are only for FieldSchema, correct? Putting them here in constants.js is a bit confusing. Without looking at the other modules, it's not easy to tell what it is for only by the name of MUTUALLY_EXCLUSIVE_FIELDS. I suggest keeping incompatibleFields in mutuallyExclusiveFields.js and import it in FieldSchema.js. If that creates a circular import, maybe we can do the opposite, i.e., initialize incompatibleFields in FieldSchema.js and import it in mutuallyExclusiveFields.js. Does that make sense?
There was a problem hiding this comment.
It does, yeah. The main issue is that the checks and schemas are each expected to export a single check/schema. We can export more things, but that requires code changes wherever schemas are imported (which is a more involved change). To that end, it would make sense to have it in its own common file, which is basically constants. I'll tweak the name and add comments to make it more clear why it's there though.
lib/schemas/FieldSchema.js
Outdated
| { | ||
| key: 'abc', | ||
| children: [{ key: 'def', children: [{ key: 'dhi' }] }], | ||
| skip: true |
There was a problem hiding this comment.
The name skip looks like part of the field definition, but it's actually an internal meta field used to bypass the example tests. Can we rename so it looks more like an "internal" thing, such as _skipTest?
There was a problem hiding this comment.
Sure! I also moved it into a constant which will be nice as soon as it comes up a bunch of places.
|
Added comments and clarity per above in f0281f9. Thanks! |
This sort of relates to PDE-152 in that I looked at this because I was looking at that, but this it it's own thing.
Examples can now be excluded from tests. This is helpful when they conform to a functional constraint, not a schema level one. Also, made an effort to surprise devs less by putting functional restrictions into the doc.