Conversation
twaite
commented
Mar 2, 2019
twaite
commented
- Adds useMemo: Create useMemo hook #3
- Adds useObservedProps
- Adds Docs
Feature/observables
…om the useMemo branch
This reverts commit 0f7b135.
| } | ||
|
|
||
| hookCallIndex++; | ||
| deps = deps != null ? deps : null; |
There was a problem hiding this comment.
I believe it has to do w/ typescript validation. @jcampuza ?
There was a problem hiding this comment.
That’s what I thought at first but the type after this statement will still be <any[] | null>. When th conditional down below checks for null, that’s when typescript will recognize it as <any[]>
There was a problem hiding this comment.
It's to force deps to be of type <any[] | null>. Before this statement it would be of type <any[] | null | undefined>. In essence it's casting it so that it's not undefined.
There was a problem hiding this comment.
React's hooks implementation does a similar check, a lot of this was inspired by that.
There was a problem hiding this comment.
Thanks! I missed the ? in the input params.
| Object.defineProperty(obj, '__ancestors', { | ||
| value: ancestors, | ||
| enumerable: false, | ||
|
|
| } | ||
| }; | ||
|
|
||
| const _handleArrayFunctions = (scope: IEmberHooksComponent, target: any, prop: string): Function => { |
There was a problem hiding this comment.
I think we can narrow down the type of target here to any[] since this should only be called with arrays.
| // TODO: tw - should this be an object instead of ifs? | ||
| const emberArray = scope.get(target.__ancestors.join()); | ||
| if (prop === 'push') { | ||
| return (val: any) => { |
There was a problem hiding this comment.
I think we may want to create separate functions for these, or at least give them names for debugging purposes, and it'd be a slight perf enhancement to not create inline functions.
|
Left a few comments, but these could also be addressed as part of another PR as well, waiting for this to get merged before starting on any other hooks. |
4 participants
