feat: ISR with auto-translated content i18n for node pages#257
feat: ISR with auto-translated content i18n for node pages#257snomiao wants to merge 73 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds ISR-time localization for node detail pages by extracting stored translations from the registry API and auto-translating missing content via OpenAI, aiming to improve SEO for localized node content.
Changes:
- Introduces
getTranslatedNodeContent+translateNodeContentto extract/polish node translations and optionally auto-translate via OpenAI. - Adds
getStaticProps/getStaticPaths(ISR) to node detail pages and passestranslatedContentinto the UI. - Updates
useNextTranslationnavigation behavior (removes shallow routing) and threadstranslatedContentintoNodeDetails.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/hooks/i18n/translateNode.ts |
New helper for extracting stored translations and auto-translating missing content via OpenAI. |
src/hooks/i18n/index.ts |
Adjusts locale switching to trigger full navigation (no shallow) so ISR content can be re-fetched. |
pages/nodes/[nodeId].tsx |
Adds ISR fetch/translation + basic SEO meta tags; passes translated content to NodeDetails. |
pages/publishers/[publisherId]/nodes/[nodeId].tsx |
Adds ISR fetch/translation and passes translated content to NodeDetails for publisher-scoped node pages. |
components/nodes/NodeDetails.tsx |
Accepts translatedContent and conditionally renders translated description based on active locale. |
docs/i18n-content-translation.md |
New doc collecting related PRs/issues and architecture notes for content translation. |
docs/i18n-content-approaches.md |
New doc comparing implementation approaches for dynamic content translation. |
Comments suppressed due to low confidence (6)
src/hooks/i18n/translateNode.ts:110
translateNodeContentparses the model reply using a regex that requires\nafter[key]. The OpenAI API may return Windows newlines (\r\n) or extra whitespace, which would cause parsing to fail and silently keep the English fallback. Consider making the parser tolerant of\r?\nand minor formatting variation so translations reliably apply.
return result;
} catch {
return content;
}
}
src/hooks/i18n/index.ts:72
router.replaceis settinghashtorouter.asPath.split('#')[1](no leading#). Elsewhere (e.g.src/hooks/useSearchParameter.ts) the hash includes the#prefix, and Next's URL/hash handling typically expects the leading#. Aligning this to include#(or usingnew URL(router.asPath, location.href).hash) will avoid losing/breaking anchor navigation when switching locales.
query: router.query,
hash: router.asPath.split("#")[1] || "",
},
router.asPath,
{ locale },
);
},
};
}
pages/nodes/[nodeId].tsx:110
getStaticPropsis fetching translated content, but the page still renders<NodeDetails />which fetches node data client-side and shows a loading spinner until React Query resolves. That means the statically generated HTML body likely won’t contain the node description/changelog, undermining the stated SSR/SEO benefits. Consider passing the fetched node (or at least description/changelog) down as props and/or hydrating React Query so the first render contains the translated content.
const NodeView = ({
nodeName,
translatedContent,
}: InferGetStaticPropsType<typeof getStaticProps>) => {
const router = useRouter();
const { nodeId } = router.query;
const { t } = useNextTranslation();
const title = nodeName ? `${nodeName} - ${t("ComfyUI Registry")}` : t("ComfyUI Registry");
const description = translatedContent?.description ?? "";
return (
<div className="p-4">
<Head>
<title>{title}</title>
{description && <meta name="description" content={description.slice(0, 160)} />}
{description && <meta property="og:description" content={description.slice(0, 160)} />}
{nodeName && <meta property="og:title" content={title} />}
</Head>
<div className="py-4">
<Breadcrumb>
<Breadcrumb.Item
href="/"
icon={HiHome}
onClick={(e) => {
e.preventDefault();
router.push("/");
}}
className="dark"
>
{t("Home")}
</Breadcrumb.Item>
<Breadcrumb.Item className="dark">{t("All Nodes")}</Breadcrumb.Item>
<Breadcrumb.Item className="dark text-blue-500">{nodeId as string}</Breadcrumb.Item>
</Breadcrumb>
</div>
<NodeDetails translatedContent={translatedContent} />
</div>
);
};
export default NodeView;
pages/nodes/[nodeId].tsx:86
- The meta description is derived from
translatedContent.description.slice(0, 160)without normalizing whitespace/newlines. Node descriptions can include line breaks or markup-like text, which can degrade snippet quality. Consider trimming, collapsing whitespace/newlines, and ideally stripping any markdown before generating the<meta name="description">/og:descriptioncontent.
{description && <meta name="description" content={description.slice(0, 160)} />}
{description && <meta property="og:description" content={description.slice(0, 160)} />}
{nodeName && <meta property="og:title" content={title} />}
</Head>
<div className="py-4">
<Breadcrumb>
<Breadcrumb.Item
href="/"
pages/publishers/[publisherId]/nodes/[nodeId].tsx:95
- This page adds ISR translation in
getStaticPropsbut does not add the localized<Head>meta tags (title/description/og tags) described in the PR summary (and implemented inpages/nodes/[nodeId].tsx). If publisher-scoped node pages are intended to have the same SEO treatment, they should also render equivalent<Head>tags using the translated content.
const NodeView = ({ translatedContent }: InferGetStaticPropsType<typeof getStaticProps>) => {
const router = useRouter();
const { publisherId, nodeId } = router.query;
const { data: publisher } = useGetPublisher(publisherId as string);
const { t } = useNextTranslation();
return (
<div className="p-4">
<Breadcrumb className="py-4">
<Breadcrumb.Item
href="/"
icon={HiHome}
onClick={(e) => {
e.preventDefault();
router.push("/");
}}
className="dark"
>
{t("Home")}
</Breadcrumb.Item>
<Breadcrumb.Item
href={`/publishers/${publisherId}`}
onClick={(e) => {
e.preventDefault();
router.push(`/publishers/${publisherId}`);
}}
className="dark"
>
{publisher?.name || publisherId}
</Breadcrumb.Item>
<Breadcrumb.Item className="text-blue-500">{nodeId as string}</Breadcrumb.Item>
</Breadcrumb>
<NodeDetails translatedContent={translatedContent} />
</div>
);
};
export default NodeView;
components/nodes/NodeDetails.tsx:565
translatedContentincludes a translatedchangelog, but the UI still renders version changelogs fromnodeVersions/version.changelogonly, so changelog translation work is currently unused. If the intent is “description + latest changelog translated”, consider applyingtranslatedContent.changelogto the latest version display (and/or the drawer) when the locale matches, or adjust the translation logic/PR description accordingly.
<div className="w-full mt-4 lg:w-1/6 ">
<div className="flex flex-col gap-4">
{node.repository && (
<Button
className="flex-shrink-0 px-4 text-white bg-blue-500 rounded whitespace-nowrap text-[16px]"
onClick={(e) => {
analytic.track("View Repository");
e.preventDefault();
window.open(node.repository, "_blank", "noopener noreferrer");
}}
href={node.repository || ""}
>
<MdOpenInNew className="w-5 h-5 mr-2" />
{t("View Repository")}
</Button>
)}
{!!node.latest_version?.downloadUrl && (
<Button
hidden={isUnclaimed}
className="flex-shrink-0 px-4 text-white bg-blue-500 rounded whitespace-nowrap text-[16px]"
onClick={(e: React.MouseEvent<HTMLButtonElement>) => {
e.preventDefault();
if (node?.latest_version?.downloadUrl) {
downloadFile(
node.latest_version?.downloadUrl,
`${node.name}_${node.latest_version.version}.zip`,
);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously the toggle label always said "Auto-translated" for any non-English
content, even when the translation came from the publisher-provided stored
translation. This was misleading.
Thread a `TranslationSource` ("original" | "stored" | "auto") through
`getTranslatedNodeContent`, `translateNodeContent`, the /api/translate-node
response, and into `ContentToggle`, which now shows:
- "Translated" (with publisher tooltip) for stored translations
- "Auto-translated" (with AI tooltip) for OpenAI-generated translations
Also harden the async translation path in `NodeDetails` to ignore the
English-fallback case the API returns on OpenAI/stored miss, so a stale
English result can no longer overwrite the ISR-provided translation.
Addresses copilot-pull-request-reviewer comment on ContentToggle.tsx.
No longer needed since getStaticProps blocks on translation (Approach B). Removes the unprotected API route that Copilot flagged for OpenAI cost. Amp-Thread-ID: https://ampcode.com/threads/T-019d5c8b-806a-73a3-a73a-fe883ee920d1 Co-authored-by: Amp <amp@ampcode.com>
…slate-node route removal The previous commit removed /api/translate-node but left references to asyncTranslation, isTranslating, and the useEffect that fetched it. This broke the build with 'Cannot find name' errors. Remove the dead async translation state, useEffect, and unused imports. Wire ContentToggle directly to ISR-provided translations only. Amp-Thread-ID: https://ampcode.com/threads/T-019d714f-cc60-7591-be98-953c468cbb54 Co-authored-by: Amp <amp@ampcode.com>
Addresses Copilot review comment — encodeURIComponent the IDs in the publisher-mismatch redirect to handle special characters safely. Amp-Thread-ID: https://ampcode.com/threads/T-019d714f-cc60-7591-be98-953c468cbb54 Co-authored-by: Amp <amp@ampcode.com>
- Only auto-translate when content.source is 'original' (English fallback), not when a stored translation happens to match the source description - Remove stale /api/translate-node reference from docstring Amp-Thread-ID: https://ampcode.com/threads/T-019d714f-cc60-7591-be98-953c468cbb54 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d714f-cc60-7591-be98-953c468cbb54 Co-authored-by: Amp <amp@ampcode.com>
…le comment - Replace VERCEL_GIT_COMMIT_REF === 'sno-i18n-isr' with PREHEAT_ENABLED env var - Remove stale /api/translate-node reference from getStaticProps comment Amp-Thread-ID: https://ampcode.com/threads/T-019d714f-cc60-7591-be98-953c468cbb54 Co-authored-by: Amp <amp@ampcode.com>
… fallback Remove references to /api/translate-node route and NodeDetails useEffect. Update architecture diagram and key decisions to reflect current state: human path uses stored translations only, bot path blocks on OpenAI. Amp-Thread-ID: https://ampcode.com/threads/T-019d714f-cc60-7591-be98-953c468cbb54 Co-authored-by: Amp <amp@ampcode.com>
…gram The "Per page" section previously listed OpenAI translation as step 3 for all paths. Moved it to the Bot path section since human path only uses stored translations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The comment previously claimed "both routes serve identical content" but bots may receive OpenAI auto-translated content while humans see stored translations or English fallback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (publisherId && node.publisher?.id && node.publisher.id !== publisherId) { | ||
| return { | ||
| redirect: { | ||
| destination: `/publishers/${encodeURIComponent(node.publisher.id)}/nodes/${encodeURIComponent(nodeId)}`, | ||
| permanent: false, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
When publisherId is provided, this only redirects on mismatch if node.publisher?.id is truthy. If the API returns publisher as null/undefined for a node, the page will render (and be ISR-cached) under whatever /publishers/{publisherId}/nodes/{nodeId} URL was requested. Consider handling the !node.publisher?.id case as well (e.g., notFound: true or redirecting to the canonical /nodes/{nodeId} route) to avoid caching content under incorrect publisher URLs.
| if (publisherId && node.publisher?.id && node.publisher.id !== publisherId) { | |
| return { | |
| redirect: { | |
| destination: `/publishers/${encodeURIComponent(node.publisher.id)}/nodes/${encodeURIComponent(nodeId)}`, | |
| permanent: false, | |
| }, | |
| }; | |
| if (publisherId) { | |
| const actualPublisherId = node.publisher?.id; | |
| if (!actualPublisherId) { | |
| return { | |
| redirect: { | |
| destination: `/nodes/${encodeURIComponent(nodeId)}`, | |
| permanent: false, | |
| }, | |
| }; | |
| } | |
| if (actualPublisherId !== publisherId) { | |
| return { | |
| redirect: { | |
| destination: `/publishers/${encodeURIComponent(actualPublisherId)}/nodes/${encodeURIComponent(nodeId)}`, | |
| permanent: false, | |
| }, | |
| }; | |
| } |
There was a problem hiding this comment.
Already implemented in current code — see src/hooks/i18n/nodeStaticProps.ts:54-61. When publisherId is provided but node.publisher?.id is falsy, the loader redirects to the canonical /nodes/${nodeId} URL with permanent: false (never caches a 200 under the bad publisher path).
| ### 4. No client-side translation fallback | ||
|
|
||
| **Decision**: Removed the `/api/translate-node` route and client-side `useEffect` translation. Human visitors see stored translations or English; bots are routed by middleware to `/_bot/nodes/[nodeId]` which blocks on OpenAI. | ||
|
|
||
| **Why**: Client-side translation couldn't populate `<meta>` tags for SEO, and added complexity. The bot path guarantees translated HTML for crawlers, while human visitors get instant page loads with stored translations. |
There was a problem hiding this comment.
The PR description still documents a human-path progressive translation flow (client calling /api/translate-node) and lists pages/api/translate-node.ts as added, but the codebase has no pages/api and this doc explicitly says that route + client fallback were removed. Please update the PR description (or reintroduce the route) so reviewers/operators don’t get conflicting guidance about how translations are generated for humans vs bots.
There was a problem hiding this comment.
Fixed — the PR description has been rewritten to match current code: no /api/translate-node route, no client-side async fallback. The Changes section now lists only files that actually ship in this PR.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
@copilot review please |
Previously a 404 from the registry API would fall through to the catch block and return generic props with revalidate: 60 — Next.js would then cache a 200 response with a spinner body under the bad URL. For the bot route this is especially bad for SEO/canonicalization. Special-case res.status === 404 to return notFound: true so Next.js serves a real 404 page (and caches that decision for an hour). Other non-2xx statuses still fall through to the props fallback for transient errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Add server-side content translation for node detail pages with a bot/human split architecture: human visitors get instant page renders from stored translations only (or English fallback), while search-engine crawlers get blocking ISR that guarantees translated meta tags in the HTML source via OpenAI auto-translation.
How it works
Human path (
/nodes/[id],/publishers/[id]/nodes/[id])getStaticPropsfetches node data + storednode.translations[locale]from the registry API — never awaits OpenAI, so cold ISR renders instantly<ContentToggle>lets users flip between translated and original English when a translation is available; preference is persisted in localStorageBot path (
/_bot/nodes/[id],/_bot/publishers/[id]/nodes/[id])middleware.tsdetects search-engine UAs (via theisbotpackage, 1000+ patterns) and rewrites the request to the/_bot/*variant — same URL to the bot, different internal routegetStaticPropsfor the bot variant awaits the OpenAI translation so the rendered HTML always contains localized<meta>tags on first crawl/_bot/...URLs is redirected back to the canonical pathrevalidate: 3600) so subsequent bot hits are served from cacheWhy a bot/human split?
The previous design awaited OpenAI in
getStaticPropsfor everyone, causing a 2-3 second blank screen on cold ISR for the first visitor of each (node, locale). The split keeps the SEO benefit (bots see translated meta tags) while delivering instant page loads to humans.Note: bots and humans may see different content for a given (node, locale) pair when no stored translation exists — bots get the OpenAI-translated text while humans get the English fallback. Both paths converge once a translation is stored upstream.
Performance
getStaticPathspreheats top N nodes × top locales at build time so most non-en visits hit a pre-rendered page. Preheat runs whenVERCEL_ENV === 'production'or whenPREHEAT_ENABLED === 'true'.SEO
<html lang>per locale<meta name="description">/og:descriptionwith translated content (bot path)Changes
Pages
pages/nodes/[nodeId].tsx— non-blockinggetStaticProps(human path) + SEO<Head>tags +getStaticPathspreheatpages/publishers/[publisherId]/nodes/[nodeId].tsx— same, with publisher mismatch redirectpages/_bot/nodes/[nodeId].tsx— new: blocking ISR variant for botspages/_bot/publishers/[publisherId]/nodes/[nodeId].tsx— new: same for publisher routeComponents
components/nodes/ContentToggle.tsx— new: switches between translated and original, persists preference, shows source-aware label (publisher vs auto)components/nodes/NodeDetails.tsx— acceptstranslatedContentprop, renders via<ContentToggle>Helpers
src/hooks/i18n/translateNode.ts— new: stored-translation extraction + OpenAI auto-translation withAbortControllertimeoutsrc/hooks/i18n/nodeStaticProps.ts— new: sharedloadNodeStaticPropsused by both human and bot routes (publisher validation, 404 → notFound, blocking flag)src/hooks/i18n/index.ts— dropshallow: trueso locale switch re-fetches ISR datasrc/constants.ts— exportLANGUAGE_NAMESMiddleware
middleware.ts— bot UA detection viaisbot+/nodes/* → /_bot/nodes/*rewrite, with reverse redirect for direct/_botaccessDocs
docs/i18n-content-translation.md/docs/i18n-content-approaches.md/docs/i18n-isr-implementation.md— design notes, related PRs/issues, lessons learnedPreview
To verify the bot path locally:
curl -A "Googlebot/2.1" https://.../ja/nodes/<id>should return HTML with translated<meta name="description">.