Skip to content

Conversation

@8eecf0d2
Copy link

#2

Initial support for arrays of objects - arrays of other types might already work once added to the primitives.

Error responses are OK but will probably need tweaking for non-object array types.

@8eecf0d2
Copy link
Author

8eecf0d2 commented Jan 24, 2018

Although part 2 of #2 isn't implemented, the lack of string[] | number[] ... primitives should throw an error at compile time. Will fix up in the morning.


if (_.isArray(fieldSpecs[specName])) {
return values[valueKey].map((arrayValue, index) => {
return getMismatchedFields(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part can be modified to support primitive arrays. I imagine the way to specify primitive arrays to be like ['number'] or ['boolean'], following your test example. Then to add primitive support here, you need to:

  1. Extract the primitive check from Line 76 to Line 111 to a function accepting the primitive type and a value.
  2. Check here if the fieldSpecs[specName][0] is a string, indicating a primitive type. If it is, use the extracted function to check. Otherwise, call getMismatchedFields.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, in the extracted function, check if the primitive type is one of the expected types, i.e. the string name is correct. You can throw a TypeError if it isn't. This is to provide runtime safety.

return values[valueKey].map((arrayValue, index) => {
return getMismatchedFields(
arrayValue,
fieldSpecs[specName][0] as Interface,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a array out-of-bound guard here. If fieldSpecs[specName][0] is undefined, throw a TypeError here. The message can be a JSON.stringified string of the whole fieldSpecs and the specName to help with debugging.

@hanlindev
Copy link
Owner

Thank you brod. This diff looks good. Although the syntax is not exactly what I imagined (it deviates from TS syntax), I do like the simplicity and potential type safety. I only have the above two change requests.

Copy link
Owner

@hanlindev hanlindev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two inline comments

@8eecf0d2
Copy link
Author

Cheers. Agreed on syntax, ['primitive'] looks better. Opted for object[] style because it was dead easy to get working. Will amend and PR when I have more.

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.

2 participants