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

fix: remove hydration errors #3262

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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: 8 additions & 1 deletion components/AlgoliaSearch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ export default function AlgoliaSearch({ children }: { children: React.ReactNode
*/
export function SearchButton({ children, indexName = INDEX_NAME, ...props }: ISearchButtonProps) {
const { onOpen, onInput } = useContext(SearchContext);
const [Children, setChildren] = useState<string | React.ReactNode>('');
const searchButtonRef = useRef<HTMLButtonElement>(null);
const actionKey = getActionKey();

Expand All @@ -308,6 +309,12 @@ export function SearchButton({ children, indexName = INDEX_NAME, ...props }: ISe
};
}, [onInput, searchButtonRef]);

useEffect(() => {
if (typeof children === 'function') {
setChildren(children({ actionKey }));
}
}, []);

return (
<button
type='button'
Expand All @@ -318,7 +325,7 @@ export function SearchButton({ children, indexName = INDEX_NAME, ...props }: ISe
{...props}
data-testid='Search-Button'
>
{typeof children === 'function' ? children({ actionKey }) : children}
{Children}
</button>
);
}
8 changes: 1 addition & 7 deletions components/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,5 @@ export default function Avatar({ name, photo, link, className }: AvatarProps) {
/>
);

return link ? (
<a href={link} data-testid='Avatars-link'>
{avatar}
</a>
) : (
<React.Fragment>{avatar}</React.Fragment>
);
return link ? <span data-testid='Avatars-link'>{avatar}</span> : <React.Fragment>{avatar}</React.Fragment>;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unnecessary Fragment

The use of <React.Fragment> is unnecessary when there's only one child element.

Simplify the code by removing the Fragment:

- return link ? <span data-testid='Avatars-link'>{avatar}</span> : <React.Fragment>{avatar}</React.Fragment>;
+ return link ? <span data-testid='Avatars-link'>{avatar}</span> : avatar;

This change aligns with the static analysis hint and improves code clarity.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return link ? <span data-testid='Avatars-link'>{avatar}</span> : <React.Fragment>{avatar}</React.Fragment>;
return link ? <span data-testid='Avatars-link'>{avatar}</span> : avatar;
🧰 Tools
🪛 Biome

[error] 34-34: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

}
13 changes: 2 additions & 11 deletions components/newsroom/FeaturedBlogPost.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,9 @@ export default function FeaturedBlogPost({ post, className = '' }: FeaturedBlogP
{post.authors
.map((author, index) =>
author.link ? (
<a
key={index}
data-alt={author.name}
href={author.link}
onClick={(e) => {
e.stopPropagation();
}}
target='_blank'
rel='noreferrer'
>
<span key={index} data-alt={author.name} rel='noreferrer'>
{author.name}
</a>
</span>
Comment on lines +90 to +92
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Multiple Components Affected by Removal of Author Link Functionality

The change to replace author <a> tags with <span> elements removing link functionality is present in multiple components:

  • components/navigation/BlogPostItem.tsx
  • components/layout/BlogLayout.tsx

Impact:

  1. User Experience:

    • Users can no longer click on author names to view their profiles across multiple sections.
  2. SEO:

    • Removal of internal links to author profiles may negatively affect SEO.
  3. Consistency:

    • Inconsistent navigation patterns across the application can confuse users.

Recommendation:

Conduct a comprehensive review of all instances where author links are rendered to assess the necessity of this change and ensure that it aligns with the project's overall goals. If the removal was intentional, consider updating all related components consistently. If not, revert the changes to maintain expected functionality.

🔗 Analysis chain

Reconsider removing author link functionality

The change from an anchor tag to a span for author names with links removes important functionality:

  1. Users can no longer click on author names to view their profiles.
  2. SEO may be negatively impacted by removing these internal links.
  3. The user experience is diminished by removing expected interactive elements.

Additionally, the removal of the onClick event handler that prevented event propagation might lead to unintended behavior, such as triggering the parent Link component's click event when clicking on an author name.

Consider reverting this change to maintain the original functionality. If there were specific issues with the previous implementation, address them directly rather than removing the feature entirely. Here's a suggested fix:

- <span key={index} data-alt={author.name} rel='noreferrer'>
-   {author.name}
- </span>
+ <a
+   key={index}
+   href={author.link}
+   target="_blank"
+   rel="noreferrer"
+   onClick={(e) => e.stopPropagation()}
+ >
+   {author.name}
+ </a>

This preserves the original functionality while ensuring that clicks on author names don't trigger the parent Link component's click event.

To ensure this change doesn't break any existing functionality, please run the following verification script:

This will help identify if similar changes need to be made elsewhere in the codebase.


Correcting the search script for author link rendering patterns


Update the search script using file extension glob patterns


Refine the search pattern to capture all relevant author link occurrences

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any other occurrences of author link rendering in the codebase

# Test: Search for similar author link rendering patterns
rg --type typescript --type tsx 'author\.link.*?\{.*?author\.name' -g '!components/newsroom/FeaturedBlogPost.tsx'

Length of output: 150


Script:

#!/bin/bash
# Description: Check for any other occurrences of author link rendering in the codebase

# Test: Search for similar author link rendering patterns
rg --type ts --type tsx 'author\.link.*?\{.*?author\.name' -g '!components/newsroom/FeaturedBlogPost.tsx'

Length of output: 135


Script:

#!/bin/bash
# Description: Check for any other occurrences of author link rendering in the codebase

# Test: Search for similar author link rendering patterns
rg -g '*.{ts,tsx}' 'author\.link.*?\{.*?author\.name' -g '!components/newsroom/FeaturedBlogPost.tsx'

Length of output: 102


Script:

#!/bin/bash
# Description: Search for any occurrences of 'author.link' or 'author.name' in the codebase excluding the specified file

# Search for 'author.link' or 'author.name' in TypeScript and TSX files
rg -g '*.{ts,tsx}' '(author\.link|author\.name)' -g '!components/newsroom/FeaturedBlogPost.tsx'

Length of output: 833

) : (
author.name
)
Expand Down
40 changes: 17 additions & 23 deletions utils/getStatic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,24 @@ import { serverSideTranslations } from 'next-i18next/serverSideTranslations';

import i18nextConfig from '../next-i18next.config';

console.log(i18nextConfig.i18n.locales);

/**
* Retrieves the internationalization paths for the supported locales.
* @returns An array of paths for each supported locale.
*/
export const getI18nPaths = () =>
i18nextConfig.i18n.locales.map((lng) => ({
params: {
lang: lng
}
}));
i18nextConfig.i18n.locales.map((lng) => ({
params: {
lang: lng
}
}));

/**
* Retrieves the static paths for Next.js.
* @returns An object containing the fallback value and an array of paths.
*/
export const getStaticPaths = () => ({
fallback: false,
paths: getI18nPaths()
fallback: false,
paths: getI18nPaths()
});

/**
Expand All @@ -30,15 +28,13 @@ export const getStaticPaths = () => ({
* @param ns - An array of namespaces to be loaded.
* @returns An object containing the internationalization props.
*/
export async function getI18nProps(ctx:any, ns = ['common']) {
const locale = ctx?.params?.lang;

console.log(locale, 'here');
const props = {
...(await serverSideTranslations(locale, ns))
};
export async function getI18nProps(ctx: any, ns = ['common']) {
const locale = ctx?.params?.lang;
const props = {
...(await serverSideTranslations(locale, ns))
};

return props;
return props;
}

/**
Expand All @@ -47,11 +43,9 @@ export async function getI18nProps(ctx:any, ns = ['common']) {
* @returns A function that retrieves the static props.
*/
export function makeStaticProps(ns = {}) {
return async function getStaticProps(ctx:any) {
console.log(ctx, 'ctx');

return {
props: await getI18nProps(ctx, ns as any)
};
return async function getStaticProps(ctx: any) {
return {
props: await getI18nProps(ctx, ns as any)
};
};
}
Loading