-
Notifications
You must be signed in to change notification settings - Fork 12
fix(view): ensure all optional properties are validated #1438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| it('Can register a view with only required files', async () => { | ||
| const view = new View({ | ||
| id: 'minimal', | ||
| name: 'Minimal view', | ||
| icon: '<svg></svg>', | ||
| getContents: () => Promise.reject(new Error('Not implemented')), | ||
| }) | ||
|
|
||
| const navigation = new Navigation() | ||
| navigation.register(view) | ||
| expect(navigation.views).toEqual([view]) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is failing without this changes
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1438 +/- ##
==========================================
- Coverage 89.47% 89.23% -0.24%
==========================================
Files 25 25
Lines 703 706 +3
Branches 194 190 -4
==========================================
+ Hits 629 630 +1
- Misses 62 63 +1
- Partials 12 13 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This fixes an issue when using the `View` class as in this case the check `'property' in view` will always return true, thus the typecheck would fail when using the view class and `navigation.register`. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
|
rebased and added the null check |
| } else if (type === 'object' && (obj[property] === null || Array.isArray(obj[property]))) { | ||
| throw new Error(`View ${property} must be an object`) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added on the rebase
This fixes an issue when using the
Viewclass as in this case the check'property' in viewwill always return true,thus the type check would fail when using the view class and
navigation.register.