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.

Sign up for GitHub

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

Jump to bottom

fix: post page hydration #3880

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix: post page hydration #3880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f9c374
fix: post page hydration
capJavert Nov 26, 2024
ace1571
Merge branch 'main' into fix-post-page-hydration
capJavert Nov 28, 2024
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
18 changes: 13 additions & 5 deletions packages/shared/src/components/post/PostHeaderActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { ReactElement, useContext } from 'react';
import React, { ReactElement, useCallback, useContext } from 'react';
import classNames from 'classnames';
import { OpenLinkIcon } from '../icons';
import {
Expand All @@ -14,7 +14,7 @@ import { PostHeaderActionsProps } from './common';
import { PostMenuOptions } from './PostMenuOptions';
import { Origin } from '../../lib/log';
import { CollectionSubscribeButton } from './collection/CollectionSubscribeButton';
import { useViewSize, ViewSize } from '../../hooks';
import { useViewSizeClient, ViewSize } from '../../hooks';

const Container = classed('div', 'flex flex-row items-center');

Expand Down Expand Up @@ -45,11 +45,11 @@ export function PostHeaderActions({
...props
}: PostHeaderActionsProps): ReactElement {
const { openNewTab } = useContext(SettingsContext);
const isLaptop = useViewSize(ViewSize.Laptop);
const isLaptop = useViewSizeClient(ViewSize.Laptop);
const readButtonText = getReadPostButtonText(post);
const isCollection = post?.type === PostType.Collection;
const isEnlarged = isFixedNavigation || isLaptop;
const ButtonWithExperiment = () => {
const ButtonWithExperiment = useCallback(() => {
return (
<SimpleTooltip
placement="bottom"
Expand All @@ -74,7 +74,15 @@ export function PostHeaderActions({
</Button>
</SimpleTooltip>
);
};
}, [
inlineActions,
isEnlarged,
onReadArticle,
openNewTab,
post.permalink,
post.sharedPost?.permalink,
readButtonText,
]);

return (
<Container {...props} className={classNames('gap-2', className)}>
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/src/components/utilities/DateFormat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const DateFormat = ({

return (
<time
title={convertedDate.toString()}
title={convertedDate.toISOString()}
className={className}
dateTime={convertedDate.toISOString()}
>
Expand Down
17 changes: 12 additions & 5 deletions packages/shared/src/contexts/BootProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,21 @@ export const BootDataProvider = ({
};

const [initialLoad, setInitialLoad] = useState<boolean>(null);
const [cachedBootData, setCachedBootData] = useState<Partial<Boot>>(() => {
const [cachedBootData, setCachedBootData] = useState<Partial<Boot>>();

useEffect(() => {
if (localBootData) {
return localBootData;
setCachedBootData(localBootData);

return;
}
Comment on lines +136 to 141
Copy link
Contributor Author

@capJavert capJavert Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So yeah, this adds another re-render top level to boot but it allows us to avoid hydration issues due to user being differenet.

Let's look at the simple example in JSX:

const Component = ({ user }) => {
  return user ? 'a' : 'b'
}

What happens now:

  1. server side user is undefined so we render b
  2. client side user gets loaded inside useMemo default value function from local storage
  3. client side renders a because user is defined (from local storage)
  4. hydration error because b does not match a
  5. react has to re-render everything client side (affects performance)

What happens with this change of adding a top level re-render

  1. server side user is undefined so we render b
  2. client side user is still undefined, useEffect run only after first client side render
  3. hydration passes b matches b
  4. useEffect executes, user is no longer undefined
  5. app re-renders
  6. react only re-paints to DOM elements related to user change

So even though top level optimization makes sense, it actually adds more work in the end.


const boot = getLocalBootData();

if (!boot) {
return null;
setCachedBootData(null);

return;
}

if (boot?.settings?.theme) {
Expand All @@ -148,8 +154,9 @@ export const BootDataProvider = ({

preloadFeedsRef.current({ feeds: boot.feeds, user: boot.user });

return boot;
});
setCachedBootData(boot);
}, [localBootData]);

const { hostGranted } = useHostStatus();
const isExtension = checkIsExtension();
const logged = cachedBootData?.user as LoggedUser;
Expand Down
17 changes: 14 additions & 3 deletions packages/shared/src/hooks/useMedia.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export default function useMedia<T>(
values: T[],
defaultValue: T,
ssrValue = defaultValue,
afterHydration = false,
): T {
const getMedia = (): MediaQueryList[] =>
queries.map((q) => window.matchMedia(q));
Expand All @@ -14,9 +15,13 @@ export default function useMedia<T>(
return values?.[index] || defaultValue;
};

const [value, setValue] = useState<T>(
typeof window !== 'undefined' ? getValue(getMedia()) : ssrValue,
);
const [value, setValue] = useState<T>(() => {
if (afterHydration) {
return undefined;
}

return typeof window !== 'undefined' ? getValue(getMedia()) : ssrValue;
});

useEffect(() => {
const mediaQueryLists = getMedia();
Expand Down Expand Up @@ -45,4 +50,10 @@ export default function useMedia<T>(
return value;
}

export const useMediaClient = <T>(
queries: string[],
values: T[],
defaultValue: T,
): T => useMedia(queries, values, defaultValue, undefined, true);

export { useMedia };
15 changes: 14 additions & 1 deletion packages/shared/src/hooks/useViewSize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useMemo } from 'react';
import { useMedia } from './useMedia';
import { useMedia, useMediaClient } from './useMedia';
import {
desktop,
desktopL,
Expand Down Expand Up @@ -46,4 +46,17 @@ const useViewSize = (size: ViewSize): boolean => {
return reversedEvaluatedSizes.includes(size) ? !check : check;
}, [check, size]);
};

export const useViewSizeClient = (size: ViewSize): boolean => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Introduced this to not break current implementations but basically due to same explanation like in https://github.com/dailydotdev/apps/pull/3880/files#r1858486018, having useState default value different in SSR and client side causes hydration issues.

const check = useMediaClient(
[viewSizeToQuery[size].replace('@media ', '')],
[true],
false,
);

return useMemo(() => {
return reversedEvaluatedSizes.includes(size) ? !check : check;
}, [check, size]);
};

export { useViewSize };
2 changes: 1 addition & 1 deletion packages/webapp/pages/posts/[id]/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ PostPage.layoutProps = {
export default PostPage;

export async function getStaticPaths(): Promise<GetStaticPathsResult> {
return { paths: [], fallback: true };
return { paths: [], fallback: 'blocking' };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will make sure we see hydration issues in development, only caveat is that first ever request in production will now stale for the amount of time it takes to fetch data from our API, and now it would show empty screen.

}

export async function getStaticProps({
Expand Down