Skip to content
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

feat: stream translation titles in feed and post page #4098

Merged
merged 17 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/extension/__tests__/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,13 @@ Object.defineProperty(global, 'open', {
value: jest.fn(),
});

Object.defineProperty(global, 'TransformStream', {
writable: true,
value: jest.fn().mockImplementation(() => ({
backpressure: jest.fn(),
readable: jest.fn(),
writable: jest.fn(),
})),
});

structuredCloneJsonPolyfill();
1 change: 1 addition & 0 deletions packages/extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"date-fns": "^2.25.0",
"date-fns-tz": "1.2.2",
"dompurify": "^2.5.4",
"fetch-event-stream": "^0.1.5",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are using this because native EventSource does not allow headers? I would rather use the query param from AI search and native EventSource, wasteful to add dependencies for this.

Copy link
Member Author

@omBratteng omBratteng Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's because it doesn't allow headers, think it's a neater solutions than token (don't really like auth tokens in params, even short lived ones 🙈) and language as query param. Package is 730B gzipped, so negligible small imo.

Copy link
Member Author

@omBratteng omBratteng Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can shave it down to 608B gzipped by using events and not their stream wrapper which just adds two headers and checks if response is OK before returning events 🤪

Copy link
Member Author

@omBratteng omBratteng Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If anything, I think we should move AI search over to use this as well. Especially now when we're starting to send smart prompts, we might encounter issues with max length of URLs in browsers.

Also because EventSource is kinda "deprecated", no browsers is going to remove it, but it's not getting any new features.

So the package is just for parsing the stream, fetch natively supports streaming.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering can we just use fetch then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the package name is a bit misleading, it's more a parser than fetch. It exposes events which is the stream parser (608B) then stream (122B) which just wraps around fetch and returns the events parsed stream.

"focus-visible": "^5.2.1",
"graphql": "^16.9.0",
"graphql-request": "^3.6.1",
Expand Down
9 changes: 9 additions & 0 deletions packages/shared/__tests__/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ Object.defineProperty(global, 'open', {
value: jest.fn(),
});

Object.defineProperty(global, 'TransformStream', {
writable: true,
value: jest.fn().mockImplementation(() => ({
backpressure: jest.fn(),
readable: jest.fn(),
writable: jest.fn(),
})),
});

jest.mock('next/router', () => ({
useRouter: jest.fn().mockImplementation(
() =>
Expand Down
1 change: 1 addition & 0 deletions packages/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
"@paddle/paddle-js": "^1.3.2",
"@tippyjs/react": "^4.2.6",
"check-password-strength": "^2.0.10",
"fetch-event-stream": "^0.1.5",
"graphql-ws": "^5.5.5",
"node-fetch": "^2.6.6",
"react-markdown": "^8.0.7",
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/src/graphql/feed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ export const FEED_POST_FRAGMENT = gql`
}
slug
clickbaitTitleDetected
translation {
title
}
}
trending
feedMeta
Expand Down
6 changes: 6 additions & 0 deletions packages/shared/src/graphql/fragments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ export const FEED_POST_INFO_FRAGMENT = gql`
}
slug
clickbaitTitleDetected
translation {
title
}
}
`;

Expand Down Expand Up @@ -269,6 +272,9 @@ export const SHARED_POST_INFO_FRAGMENT = gql`
slug
domain
clickbaitTitleDetected
translation {
title
}
}
${PRIVILEGED_MEMBERS_FRAGMENT}
${SOURCE_BASE_FRAGMENT}
Expand Down
1 change: 1 addition & 0 deletions packages/shared/src/graphql/posts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export interface Post {
bookmarkList?: BookmarkFolder;
domain?: string;
clickbaitTitleDetected?: boolean;
translation?: { title?: boolean };
}

export type RelatedPost = Pick<
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/src/hooks/post/useSmartTitle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export const useSmartTitle = (post: Post): UseSmartTitle => {
return fetchedSmartTitle
? smartTitle
: post?.title || post?.sharedPost?.title;
}, [fetchedSmartTitle, smartTitle, post]);
}, [fetchedSmartTitle, smartTitle, post?.title, post?.sharedPost?.title]);

const shieldActive = useMemo(() => {
return (
Expand Down
166 changes: 166 additions & 0 deletions packages/shared/src/hooks/translation/useTranslation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
import { useCallback, useEffect, useRef } from 'react';
import type { InfiniteData, QueryKey } from '@tanstack/react-query';
import { useQueryClient } from '@tanstack/react-query';
import { events } from 'fetch-event-stream';
import { useAuthContext } from '../../contexts/AuthContext';
import { apiUrl } from '../../lib/config';
import type { FeedData, Post } from '../../graphql/posts';
import {
updateCachedPagePost,
findIndexOfPostInData,
updatePostCache,
} from '../../lib/query';
import { useSettingsContext } from '../../contexts/SettingsContext';

export enum ServerEvents {
Connect = 'connect',
Message = 'message',
Disconnect = 'disconnect',
Error = 'error',
omBratteng marked this conversation as resolved.
Show resolved Hide resolved
}

type UseTranslation = (props: {
queryKey: QueryKey;
queryType: 'post' | 'feed';
}) => {
fetchTranslations: (id: Post[]) => void;
};

type TranslateEvent = {
id: string;
title: string;
};

const updateTranslation = (post: Post, translation: TranslateEvent): Post => {
const updatedPost = post;
if (post.title) {
updatedPost.title = translation.title;
updatedPost.translation = { title: !!translation.title };
} else {
updatedPost.sharedPost.title = translation.title;
updatedPost.sharedPost.translation = { title: !!translation.title };
}
Comment on lines +40 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this set it wrong in cache?
eg some case the title might not render but we need it for some other field?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, don't think so


return updatedPost;
};

export const useTranslation: UseTranslation = ({ queryKey, queryType }) => {
const abort = useRef<AbortController>();
const { user, accessToken, isLoggedIn } = useAuthContext();
const { flags } = useSettingsContext();
const queryClient = useQueryClient();

const { language } = user || {};
const isStreamActive = isLoggedIn && !!language;

const updateFeed = useCallback(
(translatedPost: TranslateEvent) => {
const updatePost = updateCachedPagePost(queryKey, queryClient);
const feedData =
queryClient.getQueryData<InfiniteData<FeedData>>(queryKey);
const { pageIndex, index } = findIndexOfPostInData(
feedData,
translatedPost.id,
true,
);
if (index > -1) {
updatePost(
pageIndex,
index,
updateTranslation(
feedData.pages[pageIndex].page.edges[index].node,
translatedPost,
),
);
}
},
[queryKey, queryClient],
);

const updatePost = useCallback(
(translatedPost: TranslateEvent) => {
updatePostCache(queryClient, translatedPost.id, (post) =>
updateTranslation(post, translatedPost),
);
},
[queryClient],
);

const fetchTranslations = useCallback(
async (posts: Post[]) => {
if (!isStreamActive) {
return;
}
if (posts.length === 0) {
return;
}

const postIds = posts
.filter((node) =>
node?.title
? !node?.translation?.title
: !node?.sharedPost?.translation?.title,
)
.filter((node) =>
flags.clickbaitShieldEnabled && node?.title
? !node.clickbaitTitleDetected
: !node.sharedPost?.clickbaitTitleDetected,
)
.filter(Boolean)
.map((node) => (node?.title ? node.id : node?.sharedPost.id));

if (postIds.length === 0) {
return;
}

const params = new URLSearchParams();
postIds.forEach((id) => {
params.append('id', id);
});

const response = await fetch(`${apiUrl}/translate/post/title?${params}`, {
signal: abort.current?.signal,
headers: {
Accept: 'text/event-stream',
Authorization: `Bearer ${accessToken?.token}`,
'Content-Language': language as string,
},
});

if (!response.ok) {
return;
}

// eslint-disable-next-line no-restricted-syntax
for await (const message of events(response)) {
if (message.event === ServerEvents.Message) {
const post = JSON.parse(message.data) as TranslateEvent;
if (queryType === 'feed') {
updateFeed(post);
} else {
updatePost(post);
}
}
}
},
[
accessToken?.token,
flags.clickbaitShieldEnabled,
isStreamActive,
language,
queryType,
updateFeed,
updatePost,
],
);

useEffect(() => {
abort.current = new AbortController();

return () => {
abort.current?.abort();
};
omBratteng marked this conversation as resolved.
Show resolved Hide resolved
}, []);

return { fetchTranslations };
};
12 changes: 11 additions & 1 deletion packages/shared/src/hooks/useFeed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ import { featureFeedAdTemplate } from '../lib/featureManagement';
import { cloudinaryPostImageCoverPlaceholder } from '../lib/image';
import { AD_PLACEHOLDER_SOURCE_ID } from '../lib/constants';
import { SharedFeedPage } from '../components/utilities';
import { useTranslation } from './translation/useTranslation';

interface FeedItemBase<T extends FeedItemType> {
type: T;
dataUpdatedAt: number;
}

interface AdItem extends FeedItemBase<FeedItemType.Ad> {
Expand Down Expand Up @@ -101,14 +103,18 @@ export default function useFeed<T>(
const { user, tokenRefreshed } = useContext(AuthContext);
const { isPlus } = usePlusSubscription();
const queryClient = useQueryClient();
const { fetchTranslations } = useTranslation({
queryKey: feedQueryKey,
queryType: 'feed',
});
const isFeedPreview = feedQueryKey?.[0] === RequestKey.FeedPreview;
const avoidRetry =
params?.settings?.feedName === SharedFeedPage.Custom && !isPlus;

const feedQuery = useInfiniteQuery<FeedData>({
queryKey: feedQueryKey,
queryFn: async ({ pageParam }) => {
const res = await gqlClient.request(query, {
const res = await gqlClient.request<FeedData>(query, {
...variables,
first: pageSize,
after: pageParam,
Expand All @@ -132,6 +138,8 @@ export default function useFeed<T>(
}
}

fetchTranslations(res.page.edges.map(({ node }) => node));

return res;
},
refetchOnMount: false,
Expand Down Expand Up @@ -287,6 +295,7 @@ export default function useFeed<T>(
post: node,
page: pageIndex,
index,
dataUpdatedAt: feedQuery.dataUpdatedAt,
});
});

Expand All @@ -302,6 +311,7 @@ export default function useFeed<T>(
}, [
feedQuery.data,
feedQuery.isFetching,
feedQuery.dataUpdatedAt,
settings.marketingCta,
settings.showAcquisitionForm,
placeholdersPerPage,
Expand Down
14 changes: 13 additions & 1 deletion packages/shared/src/hooks/usePostById.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
mutationKeyToContentPreferenceStatusMap,
} from './contentPreference/types';
import type { PropsParameters } from '../types';
import { useTranslation } from './translation/useTranslation';

interface UsePostByIdProps {
id: string;
Expand Down Expand Up @@ -94,13 +95,24 @@ const usePostById = ({ id, options = {} }: UsePostByIdProps): UsePostById => {
const { initialData, ...restOptions } = options;
const { tokenRefreshed } = useAuthContext();
const key = getPostByIdKey(id);
const { fetchTranslations } = useTranslation({
queryKey: key,
queryType: 'post',
});
const {
data: postById,
isError,
isPending,
} = useQuery<PostData>({
queryKey: key,
queryFn: () => gqlClient.request(POST_BY_ID_QUERY, { id }),
queryFn: async () => {
const res = await gqlClient.request<PostData>(POST_BY_ID_QUERY, { id });
if (!res.post?.translation?.title) {
fetchTranslations([res.post]);
}

return res;
},
...restOptions,
staleTime: StaleTime.Default,
enabled: !!id && tokenRefreshed,
Expand Down
9 changes: 9 additions & 0 deletions packages/shared/src/lib/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { GARMR_ERROR } from '../graphql/common';
import type { PageInfo, Connection } from '../graphql/common';
import type { EmptyObjectLiteral } from './kratos';
import type { LoggedUser } from './user';
import { PostType } from '../graphql/posts';
import type {
FeedData,
Post,
Expand Down Expand Up @@ -482,6 +483,7 @@ export const getAllCommentsQuery = (postId: string): QueryKeyReturnType[] => {
export const findIndexOfPostInData = (
data: InfiniteData<FeedData>,
id: string,
findBySharedPost = false,
): { pageIndex: number; index: number } => {
for (let pageIndex = 0; pageIndex < data.pages.length; pageIndex += 1) {
const page = data.pages[pageIndex];
Expand All @@ -490,6 +492,13 @@ export const findIndexOfPostInData = (
if (item.node.id === id) {
return { pageIndex, index };
}
if (
findBySharedPost &&
item.node.type === PostType.Share &&
item.node.sharedPost.id === id
) {
return { pageIndex, index };
}
}
}
return { pageIndex: -1, index: -1 };
Expand Down
9 changes: 9 additions & 0 deletions packages/webapp/__tests__/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ Object.defineProperty(global, 'open', {
value: jest.fn(),
});

Object.defineProperty(global, 'TransformStream', {
writable: true,
value: jest.fn().mockImplementation(() => ({
backpressure: jest.fn(),
readable: jest.fn(),
writable: jest.fn(),
})),
});

jest.mock('next/router', () => ({
useRouter: jest.fn().mockImplementation(
() =>
Expand Down
Loading
Loading