Fix for bug where refs property does not include invalid ref, if invalid reference is part of a nested file, even if includeInvalid option is set to true

[sticklerdev]

Changes:

• The bail out action for invalid references (when building metadata) was happening one step too soon - before it was recorded in the deps property. Moved the check for isValid after the dependency is recored.
• Updated test-nested.yaml to include an invalid reference in the file.
• Updated test-json-refs.js file's "expected" values.

[whitlockjc]

Can you tell me why this is important? I don't disagree but the whole purpose of the deps property was for building a graph of dependencies so that I could identify circular dependencies. Since invalid references can't be circular, it didn't seem to fit. But...I can see where it might be useful. So what's the use case?

[sticklerdev]

This is not about circularity.

Say I have an invalid reference in file A (which is my root for resolution). If I specify includeInvalid=true, then this invalid reference in file A is reported to me in the refs property, as expected and I can filter on it. But now, if the file A references file B (which has no reference back to file A - so no circularity in play here but does have an invalid reference), then the invalid reference in file B isn't reported.

On the other hand, missing references are reported correctly whether they are in the root file A or in a nested file B.

This pull request is an attempt to fix this behavior. If these nested invalids aren't reported then I can't filter them either.

Thanks.

[whitlockjc]

I know it's not about circularity, I'm explaining where the deps property came from in the first. I'm trying to understand what other cases there are. Thanks for explaining.

[sticklerdev]

The one particular use case I have is to validate the document against JSON schema but only after all references have been successfully resolved. If the resolution fails (either because of invalids or missings), then there errors are reported back to the user, instead of moving forward with the validation step. Since invalids weren't being reported, I was coming up with a false positive and moving to the validation step instead of short-circuiting.

[whitlockjc]

Can you run npm test locally? It seems like tests are failing.

[sticklerdev]

Do you know which tests? Some tests have always failed for me (I am on windows) even before the changes?

[whitlockjc]

The TravisCI build has them, linked above. Your tests shouldn't be failing, maybe there is a Windows build issue I'm not aware of. I'll try to spin up a Windows VM to test.