chore: simplify tracing of known type and required in nested rules #179
Conversation
This file contains 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
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files
|
This was referenced Jan 27, 2024
|
@kklash could u review? i can't seem to request a review via the UI |
|
@ChALkeR do we need to rebase + edit this test? https://github.com/ExodusMovement/schemasafe/pull/177/files#diff-6208744208cefe72d8f280336376b2302056b2d145203d67bd97c35f86ac9846R18 |
ChALkeR
force-pushed
the
chalker/fix-discriminator-typecheck/0
branch
from
75dd90b
to
c87a7cc
Compare
ChALkeR
force-pushed
the
chalker/fix-discriminator-typecheck/1
branch
from
d997a64
to
8057cf2
Compare
|
@mvayngrib Done! |
mvayngrib
previously approved these changes
Jan 29, 2024
kklash
previously approved these changes
Jan 31, 2024
Base automatically changed from
chalker/fix-discriminator-typecheck/0
to
master
January 31, 2024 17:35
ChALkeR
dismissed stale reviews from kklash and mvayngrib
The base branch was changed.
|
This apparently now needs a rebase and re-reviews. |
E.g. in allOf/oneOf, we just rely on the checks in parent rule,
but only for type/required checks.
In the following schema, the nested allOf will just check
for `{ required: ['yyy'] }`:
```
{
type: 'object',
required: ['xxx','zzz'],
allOf: [{
type: 'object',
required: ['xxx', 'yyy']
}]
}
```
Properties/items can't be inherited as they affect evaluation checks,
which should be isolated from the parent.
ChALkeR
force-pushed
the
chalker/fix-discriminator-typecheck/1
branch
from
8057cf2
to
af30bc0
Compare
|
Rebased |
kklash
approved these changes
Jan 31, 2024
mvayngrib
approved these changes
Jan 31, 2024
@ChALkeR maybe we want to disable that requirement? |
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.
After #177 and #178.
E.g. in allOf/oneOf, we just rely on the checks in parent rule, but only for type/required checks.
In the following schema, the nested allOf will just check for
{ required: ['yyy'] }:Properties/items can't be inherited as they affect evaluation checks, which should be isolated from the parent.
The new
if (definitelyType(...typearr)) return nullline which replaced manual check in history is now able to catch more cases, which can be seen in generated code diffs, when we got type information from a ref (compare after #178 is landed, or just review d997a64).This also will make #176 (nested discriminators) stop complaining with just a single
type: 'object'check on the top level.