-
-
Notifications
You must be signed in to change notification settings - Fork 703
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: nexti18nSetup for OG tags error #3111
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3111--asyncapi-website.netlify.app/ |
@aeworxet This is the draft PR for the issue |
@asyncapi/bounty_team |
@@ -43,7 +43,7 @@ export default function FeaturedBlogPost({ post, className = '' }: FeaturedBlogP | |||
return ( | |||
<div className={`rounded-lg ${className}`}> | |||
<article className='h-full rounded-lg'> | |||
<Link href={post.slug}> | |||
<div > |
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.
Why do we need this change?
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.
The Link was causing the hydration to fail (see screenshot)
The reason behind the hydration error was that we are nesting a <Link>
tag within <Link>
tag.
The first <Link>
Tag:
<Link href={post.slug}> |
The second (nested) <Link>
Tag:
website/components/newsroom/FeaturedBlogPost.tsx
Lines 69 to 78 in 6159306
<Link href={post.slug}> | |
<span className='block' data-testid='FeaturedBlog-title'> | |
<Heading level={HeadingLevel.h3} typeStyle={HeadingTypeStyle.smSemibold} className='mt-2'> | |
{post.title} | |
</Heading> | |
<Paragraph typeStyle={ParagraphTypeStyle.sm} className='mt-3'> | |
<TextTruncate element='span' line={2} text={post.excerpt} /> | |
</Paragraph> | |
</span> | |
</Link> |
Both of these links has the same target (i.e. post.slug). So removing the outer Ling tag should not have any side effects.
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.
<div > | |
<div> |
markdown/blog/websocket-part2.md
Outdated
@@ -165,7 +165,7 @@ servers: | |||
The API client must request an authentication "token" via the following REST API endpoint "GetWebSocketsToken" to connect to WebSockets Private endpoints. For more details, read https://support.kraken.com/hc/en-us/articles/360034437672-How-to-retrieve-a-WebSocket-authentication-token-Example-code-in-Python-3 | |||
|
|||
The resulting token must be provided in the "token" field of any new private WebSocket feed subscription: | |||
``` | |||
|
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.
Why this Codeblock is removed?
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.
Please Go to the production url https://www.asyncapi.com/blog/websocket-part2
You will see the following error message in the blog content.
Application error: a client-side exception has occurred (see the browser console for more information).
After Fixing the i18n configuration, the most part of the site started to render on the server. So any inconsistency in the markdown can be caught during the build.
When we kept the codeblocks, the following error was occuring during build
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.
But we show that code as part of codeblock. Don't we have any solution for it?
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.
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.
Sambhav has pushed solution in this commit. PTAL. Thanks.
package.json
Outdated
@@ -90,7 +92,7 @@ | |||
"react-dom": "^18", | |||
"react-ga": "^3.3.1", | |||
"react-gtm-module": "^2.0.11", | |||
"react-i18next": "^14.0.5", | |||
"react-i18next": "^14.1.3", |
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.
Do we still need this library?
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.
@sambhavgupta0705 We should uninstall the react-i18next library, i don't think we need it anymore.
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.
Yes I will fix this one
pages/[lang]/tools/cli.tsx
Outdated
// /** | ||
// * @description Get the language for the CLI page. | ||
// * @returns { paths: { params: { lang: string } }[]; fallback: boolean } | ||
// */ |
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.
// /** | |
// * @description Get the language for the CLI page. | |
// * @returns { paths: { params: { lang: string } }[]; fallback: boolean } | |
// */ |
Remove this comment.
pages/_app.tsx
Outdated
<AppContext.Provider value={{ path: router.asPath }}> | ||
{/* <MDXProvider components={mdxComponents}> */} |
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.
Why MDXProvider is commented?
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.
We are now using the MDX provider for the next.js instead of the provider from the @mdx-js/react
library.
you can find the configuration of the new next.js MDX Provider here.
When we kept provider from @mdx-js/react
. We ran into several MDX errors during the build. (see screenshot)
@sambhavgupta0705 I think we should delete the commented lines
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.
So, are we now changing the whole MDX rendering part as well?
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.
Yes correct.
Please note that, it does not affect end result. Rendered MDX will look exactly as it did with the previous MDXProvider.
@akshatnema ptal |
Signed-off-by: Sambhav Gupta <[email protected]>
Signed-off-by: Sambhav Gupta <[email protected]>
Merging this without docs approval because MD file changes are not changing any doc content. Only a few markdown syntax changes are there. |
Finally Merged @aeworxet 🚀 |
This PR fixes the OG tag error
Co-authored by : @JeelRajodiya
fixes: #2639