-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
suggestions #7382
suggestions #7382
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,6 @@ import {QueryClient} from '@tanstack/react-query' | |
import chunk from 'lodash.chunk' | ||
|
||
import {labelIsHideableOffense} from '#/lib/moderation' | ||
import * as atp from '#/types/atproto' | ||
import {precacheProfile} from '../profile' | ||
import {FeedNotification, FeedPage, NotificationType} from './types' | ||
|
||
|
@@ -256,18 +255,17 @@ function getSubjectUri( | |
return notif.uri | ||
} else if (type === 'post-like' || type === 'repost') { | ||
if ( | ||
atp.fastIsType<AppBskyFeedRepost.Record>( | ||
notif.record, | ||
AppBskyFeedRepost.isRecord, | ||
) || | ||
atp.fastIsType<AppBskyFeedLike.Record>( | ||
notif.record, | ||
AppBskyFeedLike.isRecord, | ||
) | ||
AppBskyFeedRepost.isRecord(notif.record) || | ||
AppBskyFeedLike.isRecord(notif.record) | ||
) { | ||
return typeof notif.record.subject?.uri === 'string' | ||
? notif.record.subject?.uri | ||
: undefined | ||
// Type casting here is actually safe because here we check for the | ||
// actual type bellow | ||
const record = notif.record as | ||
| AppBskyFeedRepost.Record | ||
| AppBskyFeedLike.Record | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Totally! This is actually what |
||
|
||
const uri = record.subject?.uri | ||
return typeof uri === 'string' ? uri : undefined | ||
} | ||
} else if (type === 'feedgen-like') { | ||
return notif.reasonSubject | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,9 +16,9 @@ export * as profile from '#/types/atproto/profile' | |
* } | ||
* ``` | ||
*/ | ||
export function fastIsType<R>( | ||
export function fastIsType<R extends {$type?: string}>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change prevents using |
||
record: unknown, | ||
identity: <V>(v: V) => boolean, | ||
identity: <V>(v: V) => v is V & {$type: NonNullable<R['$type']>}, | ||
Comment on lines
+19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh v nice! |
||
): record is R { | ||
return identity(record) | ||
} |
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.
I get why this makes sense and is probably the most academic way of doing this, but a few things I don't like:
embed
and the origin of the data inPostView['embed']
. The way it was typed before makes it very obvious how this component is used{$type: string}
in components like this is boilerplate I don't think we want. If we adopt this pattern, we'll be doing this in hundreds of places.