Consistent input validation, and a CHANGELOG - #2
Merged
Conversation
treeify had three different reactions to the same class of mistake. A non-string where a label belongs threw at index 0, was silently dropped anywhere else, and input that wasn't an array at all returned '' -- the quietest outcome for the input most likely to be a bug. One policy now: throw for what can't be rendered, render everything else. - Input that is not an array throws a TypeError instead of returning ''. - A non-string first element throws a TypeError, including [undefined], which used to return ''. Messages name what was received, e.g. "array-treeify: expected the first element to be a string, received number (1)". - Non-string labels are stringified instead of dropped, at any depth, and can be parent nodes. Numbers, bigints and booleans are accepted without a cast. - treeify([]) still returns '', which is a useful "nothing to render". No error class is exported; the TypeError message is the API. Valid trees render exactly as before: 15,000 renders of randomly generated string trees across all three option shapes are byte-identical to the previous implementation. The README claimed TypeScript "cannot enforce at the type level that the first element is a string", which isn't true -- a tuple type can. It would rule out building trees with push, which is the actual reason for the permissive type, so the docs now say that instead. Also documents labels and errors, with the new example covered by the README test. Adds CHANGELOG.md, backfilled from the tags, with the breaking changes above under Unreleased.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1, and the last thing before 0.2.0.
The problem
treeifyhad three different reactions to the same class of mistake:[]''''[undefined]''null,'root',{}''[1, ['child']]ErrorTypeError['root', 42, 'b']42silently dropped42renders['root', [42, 'b']]42silently dropped42rendersA number where a string belongs threw at index 0 and was silently swallowed everywhere else. Input that wasn't an array at all was the quietest outcome of the lot, even though it's the one most likely to be a real bug.
The policy
Throw for what can't be rendered, render everything else.
TypeError: array-treeify: expected an array, received nullTypeError: array-treeify: expected the first element to be a string, received number (1)treeify([])still returns''— a useful "nothing to render"No error class is exported. The
TypeErrormessage is the API.number,bigintandbooleanare now accepted as labels without a cast. The exportedTreeInputtype is unchanged.Docs
The README claimed TypeScript "cannot enforce at the type level that the first element is a string". That isn't true —
[string, ...(string | TreeInput)[]]does exactly that. It would rule out building a tree withpush, which is the real reason for the permissive type, so the docs now say that instead. Added sections on labels and errors; the new example is covered by the README test suite.CHANGELOG
Added, in Keep a Changelog format, backfilled from the tags (0.1.1 through 0.1.5). The breaking changes from this PR and from #1 are collected under
Unreleased.Verified
npm run checkclean.plain, customchars) are byte-identical to the previous implementation.