Citation prototyping experiments#26
Conversation
📝 WalkthroughWalkthroughAdds new DOI- and entity-based routing areas with layouts and pages, list and timeline UI components, and multiple DataCite-fetching helpers. Pages normalize dynamic params, concurrently fetch records/events, compute year-by-year chart data, and render two-column views with charts, event feeds, and record lists. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/components/EventFeed.tsx (1)
48-54: Removeshallowfrom these App Router links.The App Router
LinkAPI does not exposeshallow, and Next.js calls that prop Pages Router-only. Leaving it here suggests behavior that App Router navigation won't provide. (nextjs.org)Also applies to: 73-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EventFeed.tsx` around lines 48 - 54, In EventFeed.tsx remove the invalid shallow prop from the App Router Link components (the Link instance that builds the /citations/... href using event.attributes["subj-id"] and the other Link around lines 73-79); locate the Link elements in the component and delete the shallow attribute so only supported props (href, className, title, scroll, etc.) remain to avoid passing Pages-router-only props to the App Router Link.src/components/DoiRecordList.tsx (1)
47-52: Dropshallowfrom App Router links.App Router
<Link>does not supportshallow; the App Router API only listshref,replace,scroll,prefetch,onNavigate, andtransitionTypes, and Next.js explicitly documentsshallowas Pages Router-only. (nextjs.org)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DoiRecordList.tsx` around lines 47 - 52, The Link in the DoiRecordList component is using the Pages-only prop shallow; remove the shallow prop from the Link JSX (the Link element that has href={`/citations/${record.attributes.doi}`} and scroll={false}) so the App Router Link uses only supported props (href, scroll, etc.). Ensure no other Link usages in this component pass shallow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/citations/`[...doi]/page.tsx:
- Around line 27-31: The chart is receiving Facet[] (objects with id, title,
count) but ResourceTypesChart expects an array of { type, count }; locate where
doisRecords.meta.resourceTypes is passed (e.g., the call site that renders
<ResourceTypesChart ... /> and the Promise.all that fetches fetchDoisRecords)
and map the facets before passing them — transform each facet { id, title, count
} into { type: facet.id or facet.title (choose the chart's key), count:
facet.count }; apply the same mapping where other facet charts are fed (the
second occurrence noted around the later render) so all chart props match the
expected shape.
- Around line 18-24: The Page component incorrectly types and awaits params:
change the component signature to use Next.js route-aware typing
PageProps<'/citations/[...doi]'> so params.doi is typed as string[], stop using
await on params (params is not a promise), and destructure/use params.doi as an
array before calling join (e.g., const { doi } = params; const doi_id =
doi.join("/")). Update the Page function signature and references to params/doi
accordingly (look for PageProps, Page, params, doi.join).
In `@src/app/citationsByEntity/`[id]/page.tsx:
- Around line 16-18: Move the normalization/redirect logic to before any data
fetching so uppercase IDs don't hit DataCite; compute a normalizedId (e.g.,
id.toLowerCase()) at the top of the page component, and if id !== normalizedId
perform a server redirect back into the same route tree using the normalized
path (for example redirect(`/citationsByEntity/${normalizedId}`) or the
equivalent in your routing API) instead of redirecting to `/${normalizedId}`;
then call fetchEntityCitations with the normalizedId (e.g.,
fetchEntityCitations("provider.id:" + normalizedId + " OR client_id:" +
normalizedId)); apply the same change to the other normalization block that
affects the logic around the code handling lines 43-55 so all fetches use the
normalized value.
In `@src/components/DoiRecordList.tsx`:
- Around line 73-76: The UI renders undefined when a record lacks metrics;
update the rendering in DoiRecordList (the JSX that references
record.attributes.citationCount, record.attributes.viewCount, and
record.attributes.downloadCount) to default missing values (e.g., 0 or "-" as
your design) before rendering; either inline with a nullish-coalescing fallback
like record.attributes.citationCount ?? 0 or extract a small helper (e.g.,
formatMetric(record, 'citationCount')) and use that for all three fields so
undefined values never appear in the badge.
In `@src/components/EventFeed.tsx`:
- Around line 55-56: Replace substring checks using
event.attributes["subj-id"].includes(doi) with an equality comparison of
normalized DOIs: normalize both event.attributes["subj-id"] and the page doi by
stripping any "https://doi.org/" prefix (and normalizing case) then compare for
equality; update all occurrences in EventFeed.tsx where includes(doi) is used
(the instances around the current snippet and the other mentions at the blocks
corresponding to lines 63-64, 80-81, 88-89) so the check shows the checkmark
only when the normalized DOIs are exactly equal.
In `@src/data/fetch.ts`:
- Around line 501-503: The DataCite query is interpolated raw into the URL in
fetchDoisRecords which allows `&`, `=` or `#` to break the request; update
fetchDoisRecords to URL-encode the user-provided query (use encodeURIComponent
or equivalent) before inserting it into the DataCite URL and adjust the
console.log to show the encoded URL or the original plus encoded query for
debugging, and apply the same fix to the other helper that builds a DataCite URL
with a query parameter (the other function around the similar DataCite URL
construction).
---
Nitpick comments:
In `@src/components/DoiRecordList.tsx`:
- Around line 47-52: The Link in the DoiRecordList component is using the
Pages-only prop shallow; remove the shallow prop from the Link JSX (the Link
element that has href={`/citations/${record.attributes.doi}`} and
scroll={false}) so the App Router Link uses only supported props (href, scroll,
etc.). Ensure no other Link usages in this component pass shallow.
In `@src/components/EventFeed.tsx`:
- Around line 48-54: In EventFeed.tsx remove the invalid shallow prop from the
App Router Link components (the Link instance that builds the /citations/...
href using event.attributes["subj-id"] and the other Link around lines 73-79);
locate the Link elements in the component and delete the shallow attribute so
only supported props (href, className, title, scroll, etc.) remain to avoid
passing Pages-router-only props to the App Router Link.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab311e30-c5d7-4447-9883-1b412bf18201
📒 Files selected for processing (8)
src/app/citations/[...doi]/layout.tsxsrc/app/citations/[...doi]/page.tsxsrc/app/citationsByEntity/[id]/Header.tsxsrc/app/citationsByEntity/[id]/layout.tsxsrc/app/citationsByEntity/[id]/page.tsxsrc/components/DoiRecordList.tsxsrc/components/EventFeed.tsxsrc/data/fetch.ts
| interface PageProps { | ||
| params: { doi: string }; | ||
| } | ||
|
|
||
| export default async function Page({ params }: PageProps) { | ||
| const { doi } = await params; | ||
| const doi_id = doi.join("/"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the file
find . -name "page.tsx" -path "*citations*[...doi]*" 2>/dev/null | head -20Repository: datacite/puli
Length of output: 136
🏁 Script executed:
# Try alternative search
fd -t f "page.tsx" | grep -i citationsRepository: datacite/puli
Length of output: 132
🏁 Script executed:
# Broader search for the citations directory
fd -t d "citations" | head -20Repository: datacite/puli
Length of output: 102
🏁 Script executed:
# Read the actual file
cat -n src/app/citations/[...doi]/page.tsxRepository: datacite/puli
Length of output: 4004
🏁 Script executed:
# Check if there are type definitions or imports that show the proper PageProps typing
rg -t ts -t tsx "PageProps" --context 3 | head -50Repository: datacite/puli
Length of output: 84
🏁 Script executed:
# Check tsconfig or next.config to understand the project setup
cat tsconfig.json 2>/dev/null | head -50Repository: datacite/puli
Length of output: 710
🏁 Script executed:
# Look for other route handlers to see how they type params
fd -t f "page.tsx" | xargs rg "interface.*PageProps" -B2 -A2 2>/dev/null | head -100Repository: datacite/puli
Length of output: 341
🏁 Script executed:
# Search for Next.js PageProps type definitions in node_modules or types
find . -name "*.d.ts" -type f | xargs rg "PageProps" 2>/dev/null | head -20Repository: datacite/puli
Length of output: 410
🏁 Script executed:
# Check package.json for Next.js version
cat package.json | rg '"next"|"version"' -A 1 -B 1Repository: datacite/puli
Length of output: 199
🏁 Script executed:
# Check if PageProps is imported from somewhere or if it's just using a standard Next.js pattern
rg -t typescript "import.*PageProps|from.*PageProps" 2>/dev/nullRepository: datacite/puli
Length of output: 39
🏁 Script executed:
# Look at how other dynamic routes are typed
cat -n src/app/citationsByEntity/[id]/page.tsx | head -30Repository: datacite/puli
Length of output: 1486
🏁 Script executed:
# Also check the root page to see the pattern
cat -n src/app/page.tsx | head -30Repository: datacite/puli
Length of output: 1394
🏁 Script executed:
# Check if PageProps is globally defined or imported from somewhere
rg -l "PageProps" --type ts --type tsx 2>/dev/null | xargs grep -l "^import.*PageProps\|^export.*PageProps\|^type PageProps\|^interface PageProps" 2>/dev/nullRepository: datacite/puli
Length of output: 41
🏁 Script executed:
# Look for global type definitions in a types file or d.ts
fd -t f "\.d\.ts$|types\.ts" | xargs cat 2>/dev/nullRepository: datacite/puli
Length of output: 3203
🏁 Script executed:
# Check for Next.js global types or augmentations
rg "declare global|PageProps" -t typescript 2>/dev/null | head -20Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check if PageProps is coming from a global augmentation or middleware file
rg "PageProps" -B 5 -A 5 | head -80Repository: datacite/puli
Length of output: 3064
🏁 Script executed:
# Look for any explicit imports of PageProps from next or types
rg "import.*PageProps" 2>/dev/nullRepository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check if there's any global d.ts file that defines PageProps
fd "global\.d\.ts" -o fd "\.d\.ts$" | xargs cat 2>/dev/null | rg "PageProps" -B 2 -A 2Repository: datacite/puli
Length of output: 180
🏁 Script executed:
# Check if there's a next-env.d.ts or similar that might declare PageProps globally
cat -n .next/types/app/page.ts 2>/dev/null || echo "File not found"Repository: datacite/puli
Length of output: 71
🏁 Script executed:
# Search for any .d.ts files that might declare PageProps
find . -name "*.d.ts" -type f -exec cat {} \; 2>/dev/null | rg "PageProps" -B 2 -A 2Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check the package.json more thoroughly to understand the setup
cat package.jsonRepository: datacite/puli
Length of output: 1408
🏁 Script executed:
# Check Next.js version and see if PageProps is a known type in Next.js 16
# Let's search the web for Next.js 16.0.10 PageProps typing
echo "Checking the actual Next.js typing pattern in 16.0.10..."Repository: datacite/puli
Length of output: 113
🌐 Web query:
Next.js 16.0.10 PageProps type catch-all route params typing
💡 Result:
In Next.js 16.0.10 (App Router), PageProps is a global TypeScript helper type for typing page components with route-aware params and searchParams. For catch-all routes like app/shop/[...slug]/page.tsx, use PageProps with the route literal path. Example for catch-all route app/shop/[...slug]/page.tsx: tsx export default async function Page(props: PageProps<'/shop/[...slug]'>) { const { slug } = await props.params; // slug: string[] const searchParams = await props.searchParams; return
Citations:
- 1: https://dev.to/bharathkumar28/nextjs-16-type-safety-async-pageprops-typed-routes-3ilc
- 2: https://nextjs.org/docs/app/building-your-application/routing/dynamic-routes
- 3: https://nextjs.org/docs/app/api-reference/file-conventions/page
- 4: https://nextjs.org/docs/basic-features/pages
- 5: https://stackoverflow.com/questions/79119608/pageprops-type-resolution-with-basic-example
Fix the catch-all route typing before using join().
This is a [...doi] route, so doi is an array, not a string. The current interface makes doi.join("/") invalid against the page's own declared props, and Next.js 16 requires using PageProps<'/citations/[...doi]'> for route-aware params with proper typing.
🛠️ Suggested fix
-interface PageProps {
- params: { doi: string };
-}
-
-export default async function Page({ params }: PageProps) {
+export default async function Page({
+ params,
+}: PageProps<"/citations/[...doi]">) {
const { doi } = await params;
const doi_id = Array.isArray(doi) ? doi.join("/") : doi;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citations/`[...doi]/page.tsx around lines 18 - 24, The Page component
incorrectly types and awaits params: change the component signature to use
Next.js route-aware typing PageProps<'/citations/[...doi]'> so params.doi is
typed as string[], stop using await on params (params is not a promise), and
destructure/use params.doi as an array before calling join (e.g., const { doi }
= params; const doi_id = doi.join("/")). Update the Page function signature and
references to params/doi accordingly (look for PageProps, Page, params,
doi.join).
| const [record, eventsResult, doisRecords] = await Promise.all([ | ||
| fetchDoiRecord(doi_id), | ||
| fetchEvents(doi_id), | ||
| fetchDoisRecords("reference_ids:" + doi_id), | ||
| ]); |
There was a problem hiding this comment.
Map facet data into the chart's expected shape first.
ResourceTypesChart expects { type, count }[], but doisRecords.meta.resourceTypes comes through as Facet[] with { id, title, count }. Passing it straight through leaves the chart without the field it renders against.
📊 Suggested fix
const events = eventsResult?.data || [];
+ const resourceTypeData = (doisRecords.meta.resourceTypes ?? []).map(
+ ({ title, count }: { title: string; count: number }) => ({
+ type: title,
+ count,
+ }),
+ );
return (
@@
- {doisRecords.meta.resourceTypes && <ResourceTypesChart data={doisRecords.meta.resourceTypes} />}
+ {resourceTypeData.length > 0 && (
+ <ResourceTypesChart data={resourceTypeData} />
+ )}Also applies to: 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citations/`[...doi]/page.tsx around lines 27 - 31, The chart is
receiving Facet[] (objects with id, title, count) but ResourceTypesChart expects
an array of { type, count }; locate where doisRecords.meta.resourceTypes is
passed (e.g., the call site that renders <ResourceTypesChart ... /> and the
Promise.all that fetches fetchDoisRecords) and map the facets before passing
them — transform each facet { id, title, count } into { type: facet.id or
facet.title (choose the chart's key), count: facet.count }; apply the same
mapping where other facet charts are fed (the second occurrence noted around the
later render) so all chart props match the expected shape.
| const [doisRecords] = await Promise.all([ | ||
| fetchEntityCitations("provider.id:" + id + " OR client_id:" + id), | ||
| ]); |
There was a problem hiding this comment.
Normalize before fetching, and redirect back into the same route tree.
The redirect target drops the /citationsByEntity/ prefix, so an uppercase request like /citationsByEntity/FOO gets sent to /foo instead of /citationsByEntity/foo. I'd also move this normalization block ahead of the fetch on Lines 16-18 so uppercase URLs don't hit DataCite first.
↪️ Suggested fix
- redirect(`/${id.toLowerCase()}?${urlSearchParams.toString()}`);
+ const query = urlSearchParams.toString();
+ redirect(
+ `/citationsByEntity/${id.toLowerCase()}${query ? `?${query}` : ""}`,
+ );Also applies to: 43-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citationsByEntity/`[id]/page.tsx around lines 16 - 18, Move the
normalization/redirect logic to before any data fetching so uppercase IDs don't
hit DataCite; compute a normalizedId (e.g., id.toLowerCase()) at the top of the
page component, and if id !== normalizedId perform a server redirect back into
the same route tree using the normalized path (for example
redirect(`/citationsByEntity/${normalizedId}`) or the equivalent in your routing
API) instead of redirecting to `/${normalizedId}`; then call
fetchEntityCitations with the normalizedId (e.g.,
fetchEntityCitations("provider.id:" + normalizedId + " OR client_id:" +
normalizedId)); apply the same change to the other normalization block that
affects the logic around the code handling lines 43-55 so all fetches use the
normalized value.
| <span className="align-right font-semibold mt-2 bg-gray-100 text-[#003366] rounded-sm px-3 py-1 text-xs inline-block"> | ||
| Citations: <span className="font-bold mr-2">{record.attributes.citationCount} </span> | ||
| Views: <span className="font-bold mr-2">{record.attributes.viewCount} </span> | ||
| Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount} </span> |
There was a problem hiding this comment.
Default missing metrics before rendering the badge.
These fields are optional in the local type, so records without metrics will render undefined in the UI.
🪄 Suggested fix
- Citations: <span className="font-bold mr-2">{record.attributes.citationCount} </span>
- Views: <span className="font-bold mr-2">{record.attributes.viewCount} </span>
- Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount} </span>
+ Citations: <span className="font-bold mr-2">{record.attributes.citationCount ?? 0} </span>
+ Views: <span className="font-bold mr-2">{record.attributes.viewCount ?? 0} </span>
+ Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount ?? 0} </span>📝 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.
| <span className="align-right font-semibold mt-2 bg-gray-100 text-[#003366] rounded-sm px-3 py-1 text-xs inline-block"> | |
| Citations: <span className="font-bold mr-2">{record.attributes.citationCount} </span> | |
| Views: <span className="font-bold mr-2">{record.attributes.viewCount} </span> | |
| Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount} </span> | |
| <span className="align-right font-semibold mt-2 bg-gray-100 text-[`#003366`] rounded-sm px-3 py-1 text-xs inline-block"> | |
| Citations: <span className="font-bold mr-2">{record.attributes.citationCount ?? 0} </span> | |
| Views: <span className="font-bold mr-2">{record.attributes.viewCount ?? 0} </span> | |
| Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount ?? 0} </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 73 - 76, The UI renders
undefined when a record lacks metrics; update the rendering in DoiRecordList
(the JSX that references record.attributes.citationCount,
record.attributes.viewCount, and record.attributes.downloadCount) to default
missing values (e.g., 0 or "-" as your design) before rendering; either inline
with a nullish-coalescing fallback like record.attributes.citationCount ?? 0 or
extract a small helper (e.g., formatMetric(record, 'citationCount')) and use
that for all three fields so undefined values never appear in the badge.
| export async function fetchDoisRecords(query: string) { | ||
| const url = `https://api.datacite.org/dois?query=${query}&page[size]=25&include_other_registration_agencies=true&disable-facets=false&facets=resourceTypes`; | ||
| console.log("Fetching DOIs with query:", url); |
There was a problem hiding this comment.
Encode the DataCite query value before sending it upstream.
Both helpers splice a user-controlled query straight into the URL. Any &, = or # inside the DOI/search text will mutate the request instead of staying inside the query term.
🔧 Suggested fix
export async function fetchDoisRecords(query: string) {
- const url = `https://api.datacite.org/dois?query=${query}&page[size]=25&include_other_registration_agencies=true&disable-facets=false&facets=resourceTypes`;
+ const url = `https://api.datacite.org/dois?${new URLSearchParams({
+ query,
+ "page[size]": "25",
+ include_other_registration_agencies: "true",
+ "disable-facets": "false",
+ facets: "resourceTypes",
+ }).toString()}`;
console.log("Fetching DOIs with query:", url);
const response = await fetch(url, {
method: "GET",
headers: { accept: "application/vnd.api+json" },
});
@@
export async function fetchEntityCitations(query: string) {
- const url = `https://api.datacite.org/dois?query=${query}&page[size]=25&disable-facets=false&facets=citations&sort=-citation-count`;
+ const url = `https://api.datacite.org/dois?${new URLSearchParams({
+ query,
+ "page[size]": "25",
+ "disable-facets": "false",
+ facets: "citations",
+ sort: "-citation-count",
+ }).toString()}`;
console.log("Fetching DOIs with query:", url);
const response = await fetch(url, {
method: "GET",
headers: { accept: "application/vnd.api+json" },
});Also applies to: 516-518
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/data/fetch.ts` around lines 501 - 503, The DataCite query is interpolated
raw into the URL in fetchDoisRecords which allows `&`, `=` or `#` to break the
request; update fetchDoisRecords to URL-encode the user-provided query (use
encodeURIComponent or equivalent) before inserting it into the DataCite URL and
adjust the console.log to show the encoded URL or the original plus encoded
query for debugging, and apply the same fix to the other helper that builds a
DataCite URL with a query parameter (the other function around the similar
DataCite URL construction).
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/app/citationsByEntity/[id]/page.tsx (1)
57-58:⚠️ Potential issue | 🟡 MinorUse
notFound()instead of returningnullfor missing entities.Returning
nullrenders a blank page. The layout already usesnotFound()for this case — keep the 404 handling consistent.Proposed fix
+import { notFound, redirect } from "next/navigation"; -import { redirect } from "next/navigation"; const entity = await fetchEntity(id); - if (!entity) return null; + if (!entity) notFound();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/citationsByEntity/`[id]/page.tsx around lines 57 - 58, The code currently returns null when fetchEntity(id) yields no result (const entity = await fetchEntity(id); if (!entity) return null;); replace this with calling Next's notFound() to trigger a 404 (if (!entity) notFound();) and ensure notFound is imported from 'next/navigation' at the top of the file; keep the rest of the component unchanged so 404 handling is consistent with the layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/citationsByEntity/`[id]/page.tsx:
- Around line 60-62: The JSX in the page component contains nested <main>
elements which breaks HTML semantics; locate the returned markup in
src/app/citationsByEntity/[id]/page.tsx (the page component render/return) and
replace the inner <main className="w-full mx-auto flex flex-row items-start
gap-8"> with a semantically appropriate container such as a <div> or <section>,
preserving the existing className and structure so only the tag changes.
- Around line 28-30: The code computing yearNums and minYear can produce NaN
when parseInt returns NaN for non-numeric mapped entries; update the logic that
builds yearNums (used in mapped.map and variable yearNums) to parse each
item.year safely (e.g., Number or parseInt) and filter out non-finite results
before calling Math.min, then compute minYear from the filtered array and fall
back to a safe default (e.g., maxYear or new Date().getFullYear()) if the
filtered array is empty; ensure references to yearNums, mapped, minYear, and
maxYear are adjusted accordingly so the subsequent year loop never receives NaN
values.
In `@src/components/DoiRecordList.tsx`:
- Around line 57-59: The JSX directly reads record.attributes.titles[0].title
which will throw if titles is empty; in DoiRecordList update the render to guard
against empty or missing titles (e.g., use optional chaining and a safe fallback
like record.attributes.titles?.[0]?.title || 'Untitled' or fall back to another
attribute) so the <div className="font-bold text-[`#243B54`]"> never tries to
access index 0 of an empty array; keep the change local to the component where
record.attributes.titles is referenced.
- Around line 63-65: In DoiRecordList.tsx fix the metadata line rendering by
building the pieces from record.attributes (publicationYear, publisher) into an
array, filtering out falsy values, and joining them with ' · ' so orphan
separators don't appear; then separately render the agency only when present
(e.g., compute agencyLabel = agency ? 'via ' +
capitalize(record.attributes.agency) : undefined) and append it with a preceding
separator if needed—update the JSX that currently uses
{record.attributes.publicationYear} · {record.attributes.publisher} · via
{record.attributes.agency && ...} to use these conditional pieces so missing
fields produce no extraneous separators or trailing "via ".
- Around line 50-55: The Link usage in DoiRecordList.tsx passes a removed prop
`shallow` — remove the `shallow` attribute from the <Link> element that
navigates to `/citations/${record.attributes.doi}`; simply delete the `shallow`
prop (keeping existing href, className, and scroll as-is) and, if you need App
Router-specific client-side behavior later, use next/navigation hooks (e.g.,
useRouter/push or router.replace) instead of `shallow`.
---
Duplicate comments:
In `@src/app/citationsByEntity/`[id]/page.tsx:
- Around line 57-58: The code currently returns null when fetchEntity(id) yields
no result (const entity = await fetchEntity(id); if (!entity) return null;);
replace this with calling Next's notFound() to trigger a 404 (if (!entity)
notFound();) and ensure notFound is imported from 'next/navigation' at the top
of the file; keep the rest of the component unchanged so 404 handling is
consistent with the layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0f30762-4026-4a13-86bb-a078ea3624c6
📒 Files selected for processing (4)
src/app/citations/[...doi]/layout.tsxsrc/app/citations/[...doi]/page.tsxsrc/app/citationsByEntity/[id]/page.tsxsrc/components/DoiRecordList.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/citations/[...doi]/layout.tsx
- src/app/citations/[...doi]/page.tsx
| const yearNums = mapped.map((item: { year: string; count: number }) => parseInt(item.year, 10)); | ||
| const minYear = Math.min(...yearNums); | ||
| const maxYear = new Date().getFullYear(); |
There was a problem hiding this comment.
Guard against invalid year values that produce NaN.
If the API returns a non-numeric year string, parseInt returns NaN, causing Math.min(...yearNums) to return NaN and the subsequent loop to silently fail.
Proposed fix
- const yearNums = mapped.map((item: { year: string; count: number }) => parseInt(item.year, 10));
- const minYear = Math.min(...yearNums);
- const maxYear = new Date().getFullYear();
+ const yearNums = mapped
+ .map((item: { year: string; count: number }) => parseInt(item.year, 10))
+ .filter((y) => !Number.isNaN(y));
+ if (yearNums.length === 0) {
+ // No valid years, leave chartData empty
+ } else {
+ const minYear = Math.min(...yearNums);
+ const maxYear = new Date().getFullYear();
+ const yearToCount: Record<string, number> = {};
+ mapped.forEach((item: { year: string; count: number }) => {
+ yearToCount[item.year] = item.count;
+ });
+ for (let y = minYear; y <= maxYear; y++) {
+ chartData.push({
+ year: y.toString(),
+ count: yearToCount[y.toString()] ?? 0,
+ });
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citationsByEntity/`[id]/page.tsx around lines 28 - 30, The code
computing yearNums and minYear can produce NaN when parseInt returns NaN for
non-numeric mapped entries; update the logic that builds yearNums (used in
mapped.map and variable yearNums) to parse each item.year safely (e.g., Number
or parseInt) and filter out non-finite results before calling Math.min, then
compute minYear from the filtered array and fall back to a safe default (e.g.,
maxYear or new Date().getFullYear()) if the filtered array is empty; ensure
references to yearNums, mapped, minYear, and maxYear are adjusted accordingly so
the subsequent year loop never receives NaN values.
| return ( | ||
| <main className=""> | ||
| <main className="w-full mx-auto flex flex-row items-start gap-8"> |
There was a problem hiding this comment.
Avoid nested <main> elements — use a single <main> landmark per page.
Nested <main> elements violate HTML semantics and can confuse assistive technologies. Replace the inner <main> with a <div> or <section>.
Proposed fix
return (
- <main className="">
- <main className="w-full mx-auto flex flex-row items-start gap-8">
+ <main className="w-full mx-auto flex flex-row items-start gap-8">
<div className="w-1/2">
...
</div>
<div className="w-1/2">
...
</div>
- </main>
</main>
);📝 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.
| return ( | |
| <main className=""> | |
| <main className="w-full mx-auto flex flex-row items-start gap-8"> | |
| return ( | |
| <main className="w-full mx-auto flex flex-row items-start gap-8"> | |
| <div className="w-1/2"> | |
| ... | |
| </div> | |
| <div className="w-1/2"> | |
| ... | |
| </div> | |
| </main> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citationsByEntity/`[id]/page.tsx around lines 60 - 62, The JSX in the
page component contains nested <main> elements which breaks HTML semantics;
locate the returned markup in src/app/citationsByEntity/[id]/page.tsx (the page
component render/return) and replace the inner <main className="w-full mx-auto
flex flex-row items-start gap-8"> with a semantically appropriate container such
as a <div> or <section>, preserving the existing className and structure so only
the tag changes.
| <Link | ||
| href={`/citations/${record.attributes.doi}`} | ||
| className="cursor-pointer" | ||
| scroll={false} | ||
| shallow | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Next.js 16 Link component support the shallow prop?
💡 Result:
No, the Next.js 16 Link component does not support the shallow prop in the App Router (the default and recommended router). The shallow prop was specific to the Pages Router, where it enabled changing the URL without re-running data fetching methods like getStaticProps or getServerSideProps. In Next.js 16, official App Router Link documentation lists props like href, replace, scroll, prefetch, and onNavigate, but not shallow. GitHub discussions confirm shallow routing is unavailable in the App Router since Next.js 13, with no restoration in version 16 release notes. The Pages Router documentation still mentions shallow, but Next.js 16 emphasizes the App Router, and Pages Router is legacy.
Citations:
- 1: Shallow prop with link component redirect to as prop value with app directory. vercel/next.js#50274
- 2: https://nextjs.org/docs/pages/api-reference/components/link
- 3: https://beta.nextjs.org/docs/api-reference/components/link
- 4: Shallow routing on Next.js 13's next/navigation vercel/next.js#48110
- 5: https://nextjs.org/blog/next-16
- 6: https://nextjs.org/docs/app/guides/upgrading/version-16
- 7: https://nextjs.org/docs/app/getting-started/linking-and-navigating
- 8: https://nextjs.org/docs/13/app/building-your-application/routing/linking-and-navigating
- 9: https://nextjs.org/docs/app/building-your-application/routing/linking-and-navigating
🏁 Script executed:
# Check Next.js version in package.json
cat package.json | grep -A 2 '"next"'Repository: datacite/puli
Length of output: 128
🏁 Script executed:
# Check directory structure to determine routing system
git ls-files | head -20
find . -type d -name "app" -o -type d -name "pages" | head -10Repository: datacite/puli
Length of output: 515
🏁 Script executed:
# Check the actual file to see routing context
cat -n src/components/DoiRecordList.tsx | head -60Repository: datacite/puli
Length of output: 1987
Remove the shallow prop — it was removed from next/link in Next.js 13+.
The shallow prop no longer exists on the Link component in the App Router. For client-side navigation without refetching server data, the App Router handles this differently through its caching mechanisms.
Proposed fix
<Link
href={`/citations/${record.attributes.doi}`}
className="cursor-pointer"
scroll={false}
- shallow
>📝 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.
| <Link | |
| href={`/citations/${record.attributes.doi}`} | |
| className="cursor-pointer" | |
| scroll={false} | |
| shallow | |
| > | |
| <Link | |
| href={`/citations/${record.attributes.doi}`} | |
| className="cursor-pointer" | |
| scroll={false} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 50 - 55, The Link usage in
DoiRecordList.tsx passes a removed prop `shallow` — remove the `shallow`
attribute from the <Link> element that navigates to
`/citations/${record.attributes.doi}`; simply delete the `shallow` prop (keeping
existing href, className, and scroll as-is) and, if you need App Router-specific
client-side behavior later, use next/navigation hooks (e.g., useRouter/push or
router.replace) instead of `shallow`.
| <div className="font-bold text-[#243B54]"> | ||
| {record.attributes.titles[0].title} | ||
| </div> |
There was a problem hiding this comment.
Guard against empty titles array to prevent runtime crash.
titles is a required array in the type, but the API could return an empty array. Accessing titles[0].title without a check will throw a TypeError.
Proposed fix
<div className="font-bold text-[`#243B54`]">
- {record.attributes.titles[0].title}
+ {record.attributes.titles[0]?.title ?? "Untitled"}
</div>📝 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.
| <div className="font-bold text-[#243B54]"> | |
| {record.attributes.titles[0].title} | |
| </div> | |
| <div className="font-bold text-[`#243B54`]"> | |
| {record.attributes.titles[0]?.title ?? "Untitled"} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 57 - 59, The JSX directly
reads record.attributes.titles[0].title which will throw if titles is empty; in
DoiRecordList update the render to guard against empty or missing titles (e.g.,
use optional chaining and a safe fallback like
record.attributes.titles?.[0]?.title || 'Untitled' or fall back to another
attribute) so the <div className="font-bold text-[`#243B54`]"> never tries to
access index 0 of an empty array; keep the change local to the component where
record.attributes.titles is referenced.
| <div className="text-gray-500 text-sm mt-2"> | ||
| {record.attributes.publicationYear} · {record.attributes.publisher} · via {record.attributes.agency && record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)} | ||
| </div> |
There was a problem hiding this comment.
Handle missing metadata fields to avoid rendering artifacts like "· via ".
When publicationYear, publisher, or agency are undefined, the current template renders orphaned separators or "via " with nothing following. Use conditional rendering or template only the fields that exist.
Proposed fix
- <div className="text-gray-500 text-sm mt-2">
- {record.attributes.publicationYear} · {record.attributes.publisher} · via {record.attributes.agency && record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)}
- </div>
+ <div className="text-gray-500 text-sm mt-2">
+ {[
+ record.attributes.publicationYear,
+ record.attributes.publisher,
+ record.attributes.agency &&
+ `via ${record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)}`,
+ ]
+ .filter(Boolean)
+ .join(" · ")}
+ </div>📝 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.
| <div className="text-gray-500 text-sm mt-2"> | |
| {record.attributes.publicationYear} · {record.attributes.publisher} · via {record.attributes.agency && record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)} | |
| </div> | |
| <div className="text-gray-500 text-sm mt-2"> | |
| {[ | |
| record.attributes.publicationYear, | |
| record.attributes.publisher, | |
| record.attributes.agency && | |
| `via ${record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)}`, | |
| ] | |
| .filter(Boolean) | |
| .join(" · ")} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 63 - 65, In DoiRecordList.tsx
fix the metadata line rendering by building the pieces from record.attributes
(publicationYear, publisher) into an array, filtering out falsy values, and
joining them with ' · ' so orphan separators don't appear; then separately
render the agency only when present (e.g., compute agencyLabel = agency ? 'via '
+ capitalize(record.attributes.agency) : undefined) and append it with a
preceding separator if needed—update the JSX that currently uses
{record.attributes.publicationYear} · {record.attributes.publisher} · via
{record.attributes.agency && ...} to use these conditional pieces so missing
fields produce no extraneous separators or trailing "via ".
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/components/DoiRecordList.tsx (2)
57-59:⚠️ Potential issue | 🟠 MajorGuard the first title lookup.
record.attributes.titles[0].titlewill throw if the API returns an emptytitlesarray.Suggested fix
<div className="font-bold text-[`#243B54`]"> - {record.attributes.titles[0].title} + {record.attributes.titles?.[0]?.title ?? "Untitled"} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DoiRecordList.tsx` around lines 57 - 59, The code in DoiRecordList.tsx directly accesses record.attributes.titles[0].title which will throw if titles is empty or undefined; update the rendering in the DoiRecordList component to safely guard this lookup (e.g., check that record.attributes?.titles?.length > 0 or use optional chaining and a fallback) and render a sensible default (like an empty string or "Untitled") when no title exists so the component doesn't crash on empty titles arrays.
50-55:⚠️ Potential issue | 🟠 MajorRemove the unsupported
shallowprop from thisLink.
next/linkin the App Router no longer acceptsshallow, so this is ignored at best and a type/build failure at worst.Suggested fix
<Link href={`/dois/${record.attributes.doi}`} className="cursor-pointer" scroll={false} - shallow >In Next.js 16 App Router, does `next/link` support a `shallow` prop on `<Link>`?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DoiRecordList.tsx` around lines 50 - 55, The Link usage in DoiRecordList.tsx passes an unsupported shallow prop to next/link; remove the shallow prop from the Link component that wraps href={`/dois/${record.attributes.doi}`}, leaving href, className and scroll as needed, so Link no longer includes shallow and the component compiles with the App Router.src/app/citationsByEntity/[id]/page.tsx (1)
16-18:⚠️ Potential issue | 🟠 MajorMove
idnormalization ahead of the layout-level entity fetch.This redirect is too late to protect uppercase IDs.
src/app/citationsByEntity/[id]/layout.tsxalready resolves the sameidviafetchEntity(id), so an uppercase request can 404 there before this page executes. The current redirect target also drops the/citationsByEntity/prefix. Normalize in the layout (or a shared helper used by both layout and page), then use that normalized value for bothfetchEntityCitations(...)and the redirect target.Also applies to: 43-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/citationsByEntity/`[id]/page.tsx around lines 16 - 18, Normalize the route parameter before the layout-level fetch so uppercase IDs can't cause a 404: extract the normalization logic into a shared helper (e.g., normalizeEntityId) or perform toLowerCase() in the layout component that calls fetchEntity(id), then use that same normalized value in the page where fetchEntityCitations(...) is called and for the redirect target (ensure the redirect preserves the /citationsByEntity/ prefix). Update layout.tsx to call fetchEntity(normalizeEntityId(id)) (or normalize then pass the normalized id down) and update page.tsx to call fetchEntityCitations("provider.id:" + normalizedId + " OR client_id:" + normalizedId) and to build the redirect URL using /citationsByEntity/{normalizedId}.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/dois/`[...doi]/page.tsx:
- Around line 30-34: The fetchDoiRecord call can throw for 404s and currently
bubbles up as a server error; wrap the DOI lookup so 404s map to notFound()
while other errors still propagate: call fetchDoiRecord(doi_id) inside a
try/catch (or run Promise.allSettled for the three calls and inspect the
fetchDoiRecord result) and if the caught error indicates a 404 then return
notFound(), otherwise rethrow the error; keep the surrounding Promise.all usage
for fetchEvents and fetchDoisRecords (or reconstruct the Promise.all with the
resolved fetchDoiRecord value) and reference fetchDoiRecord, fetchEvents,
fetchDoisRecords, Promise.all, and notFound() to locate and change the logic.
- Around line 41-57: Filter and validate numeric years before computing minYear:
transform citationsOverTime into numeric years using parseInt (as you already do
for yearNums), then remove NaN entries (e.g., filter yearNums for
Number.isFinite or !Number.isNaN) and only call Math.min on the filtered array;
if the filtered array is empty, skip building the year loop or set a sensible
fallback; update the code that produces yearNums, minYear, and the for loop that
fills chartData to use this validated set so non-numeric years no longer produce
NaN and the chart rows are generated correctly.
- Around line 21-27: The PageProps currently types params.doi as string which is
incorrect for the catch-all route; update PageProps so params.doi is typed as
string[] (e.g., interface PageProps { params: { doi: string[] } }) and in the
Page function treat params.doi as an array (remove the unnecessary await on
params and the Array.isArray check), then compute doi_id by joining the array
(use params.doi.join("/") inside Page) so Page, PageProps, params.doi, and
doi_id reflect the correct types and logic.
---
Duplicate comments:
In `@src/app/citationsByEntity/`[id]/page.tsx:
- Around line 16-18: Normalize the route parameter before the layout-level fetch
so uppercase IDs can't cause a 404: extract the normalization logic into a
shared helper (e.g., normalizeEntityId) or perform toLowerCase() in the layout
component that calls fetchEntity(id), then use that same normalized value in the
page where fetchEntityCitations(...) is called and for the redirect target
(ensure the redirect preserves the /citationsByEntity/ prefix). Update
layout.tsx to call fetchEntity(normalizeEntityId(id)) (or normalize then pass
the normalized id down) and update page.tsx to call
fetchEntityCitations("provider.id:" + normalizedId + " OR client_id:" +
normalizedId) and to build the redirect URL using
/citationsByEntity/{normalizedId}.
In `@src/components/DoiRecordList.tsx`:
- Around line 57-59: The code in DoiRecordList.tsx directly accesses
record.attributes.titles[0].title which will throw if titles is empty or
undefined; update the rendering in the DoiRecordList component to safely guard
this lookup (e.g., check that record.attributes?.titles?.length > 0 or use
optional chaining and a fallback) and render a sensible default (like an empty
string or "Untitled") when no title exists so the component doesn't crash on
empty titles arrays.
- Around line 50-55: The Link usage in DoiRecordList.tsx passes an unsupported
shallow prop to next/link; remove the shallow prop from the Link component that
wraps href={`/dois/${record.attributes.doi}`}, leaving href, className and
scroll as needed, so Link no longer includes shallow and the component compiles
with the App Router.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f2f64a6-3d8d-47c2-b829-744b55c329fa
📒 Files selected for processing (6)
src/app/citationsByEntity/[id]/page.tsxsrc/app/dois/[...doi]/layout.tsxsrc/app/dois/[...doi]/page.tsxsrc/components/DoiRecordList.tsxsrc/components/EventFeed.tsxsrc/data/fetch.ts
✅ Files skipped from review due to trivial changes (1)
- src/components/EventFeed.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/data/fetch.ts
| interface PageProps { | ||
| params: { doi: string }; | ||
| } | ||
|
|
||
| export default async function Page({ params }: PageProps) { | ||
| const { doi } = await params; | ||
| const doi_id = Array.isArray(doi) ? doi.join("/") : doi; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Next.js 16 App Router, for a route app/dois/[...doi]/page.tsx, is paramsasync and isparams.doityped asstring[]?
💡 Result:
Yes, in Next.js 16 App Router, for a route app/dois/[...doi]/page.tsx, the params prop is async (a Promise that must be awaited), and after awaiting, params.doi is typed as string[]. To use it: tsx // app/dois/[...doi]/page.tsx export default async function Page({ params, }: { params: Promise<{ doi: string[] }>; }) { const { doi } = await params; // doi is string[] return
Citations:
- 1: https://nextjs.org/docs/app/building-your-application/routing/dynamic-routes
- 2: https://nextjs.org/docs/app/api-reference/file-conventions/dynamic-routes
- 3: https://nextjs.org/blog/next-16
- 4: https://nextjs.org/docs/app/guides/upgrading/version-16
🏁 Script executed:
# Search for PageProps type definition to see if it's a generic helper
rg "interface PageProps|type PageProps" --type tsx --type tsRepository: datacite/puli
Length of output: 84
🏁 Script executed:
# Check the file in question to see current state and surrounding context
cat -n src/app/dois/[...doi]/page.tsx | head -50Repository: datacite/puli
Length of output: 2287
🏁 Script executed:
# Search for PageProps definition more broadly without specifying file type
rg "type PageProps|interface PageProps" -A 2Repository: datacite/puli
Length of output: 362
🏁 Script executed:
# Check if there's a types file or utility defining PageProps generically
find . -name "*.ts" -o -name "*.js" -o -name "*.tsx" -o -name "*.jsx" | xargs grep -l "PageProps.*\<" | head -10Repository: datacite/puli
Length of output: 122
🏁 Script executed:
# Look at the chart generation logic to see if there's a NaN issue
cat -n src/app/dois/[...doi]/page.tsx | sed -n '40,80p'Repository: datacite/puli
Length of output: 1815
🏁 Script executed:
# Check if there's a PageProps type utility in a types/lib file
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "export.*PageProps" 2>/dev/null | head -5Repository: datacite/puli
Length of output: 146
🏁 Script executed:
# Look at other page components to see the pattern they use
cat -n src/app/citationsByEntity/[id]/page.tsx | head -30Repository: datacite/puli
Length of output: 1486
🏁 Script executed:
# Find where PageProps is defined as a generic type
rg "export.*type PageProps|declare.*PageProps" -A 5Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check the imports in citationsByEntity/[id]/page.tsx to see where PageProps comes from
cat -n src/app/citationsByEntity/[id]/page.tsx | head -15Repository: datacite/puli
Length of output: 748
🏁 Script executed:
# Search for PageProps in types directory or lib
find . -path ./node_modules -prune -o -type f \( -name "*.ts" -o -name "*.tsx" \) -print | xargs grep -l "PageProps.*=" | head -10Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Search for all imports of PageProps
rg "import.*PageProps|from.*PageProps"Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check if PageProps is in a global types file
find . -name "global.d.ts" -o -name "globals.d.ts" -o -name "types.d.ts" | xargs cat 2>/dev/nullRepository: datacite/puli
Length of output: 39
🏁 Script executed:
# Look at the full citationsByEntity/[id]/page.tsx to see if PageProps is imported
cat -n src/app/citationsByEntity/[id]/page.tsx | head -50Repository: datacite/puli
Length of output: 2274
🏁 Script executed:
# Check tsconfig or package.json for type declarations
cat tsconfig.json 2>/dev/null | jq '.compilerOptions.types' 2>/dev/null || cat tsconfig.jsonRepository: datacite/puli
Length of output: 61
🏁 Script executed:
# Look for where PageProps might come from - check `@types/next` or similar
grep -r "PageProps" package.json yarn.lock 2>/dev/null | head -20Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Search the entire codebase for any definition or declaration of PageProps, including comments
rg "PageProps" -B 2 -A 3 -uRepository: datacite/puli
Length of output: 2242
🏁 Script executed:
# Look for global type declarations or ambient type files
find . -name "*.d.ts" -o -name "*ambient*" | xargs cat 2>/dev/null | head -100Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check if PageProps is defined in a types folder or lib
find . -type d -name "types" -o -name "lib" | head -5Repository: datacite/puli
Length of output: 66
🏁 Script executed:
# Look for how page.tsx imports PageProps
cat -n src/app/page.tsx | head -20Repository: datacite/puli
Length of output: 1055
🏁 Script executed:
# Check the src/lib directory
find ./src/lib -type f | head -20Repository: datacite/puli
Length of output: 94
🏁 Script executed:
# Look for any .d.ts files in the project
find . -name "*.d.ts" 2>/dev/nullRepository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check tsconfig.json to see if there are any type roots or paths configured
cat tsconfig.jsonRepository: datacite/puli
Length of output: 710
🏁 Script executed:
# Check if PageProps is defined in next-env.d.ts (which is auto-generated)
cat next-env.d.ts 2>/dev/null || echo "File not found"Repository: datacite/puli
Length of output: 71
🏁 Script executed:
# Search all files including .d.ts for PageProps definition
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.d.ts" \) -exec grep -l "PageProps" {} \; 2>/dev/nullRepository: datacite/puli
Length of output: 212
🏁 Script executed:
# Check if there's a types file that exports PageProps
cat ./src/lib/utils.tsRepository: datacite/puli
Length of output: 225
🏁 Script executed:
# Check package.json for Next.js version and dependencies
cat package.json | grep -A 5 -B 5 "next"Repository: datacite/puli
Length of output: 671
🏁 Script executed:
# Verify that PageProps is used without import in other files
grep -n "PageProps" src/app/page.tsx src/app/\[id\]/page.tsx -B 10 | head -40Repository: datacite/puli
Length of output: 1167
🏁 Script executed:
# Check if PageProps<T> is provided by Next.js types in node_modules
grep -r "type PageProps" node_modules/next 2>/dev/null | head -5Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Let's verify the exact typing by checking what the correct catch-all route should be
cat -n src/app/citations/[...doi]/page.tsx | head -30Repository: datacite/puli
Length of output: 1333
Type params.doi correctly as string[] for this catch-all route.
In Next.js 16 App Router, [...doi] routes provide params.doi as an array, not a string. The current PageProps interface hides this contract, making the Array.isArray check on line 27 unnecessary.
Suggested fix
-interface PageProps {
- params: { doi: string };
-}
-
-export default async function Page({ params }: PageProps) {
+export default async function Page({
+ params,
+}: PageProps<"/dois/[...doi]">) {
const { doi } = await params;
- const doi_id = Array.isArray(doi) ? doi.join("/") : doi;
+ const doi_id = doi.join("/");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/dois/`[...doi]/page.tsx around lines 21 - 27, The PageProps currently
types params.doi as string which is incorrect for the catch-all route; update
PageProps so params.doi is typed as string[] (e.g., interface PageProps {
params: { doi: string[] } }) and in the Page function treat params.doi as an
array (remove the unnecessary await on params and the Array.isArray check), then
compute doi_id by joining the array (use params.doi.join("/") inside Page) so
Page, PageProps, params.doi, and doi_id reflect the correct types and logic.
| const [record, eventsResult, doisRecords] = await Promise.all([ | ||
| fetchDoiRecord(doi_id), | ||
| fetchEvents(doi_id), | ||
| fetchDoisRecords("reference_ids:" + doi_id), | ||
| ]); |
There was a problem hiding this comment.
Convert missing DOI records into notFound() instead of a server error.
fetchDoiRecord() throws on non-OK responses, so a nonexistent DOI currently bubbles out as an unhandled error. This route should map 404s to notFound() and only rethrow genuine upstream failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/dois/`[...doi]/page.tsx around lines 30 - 34, The fetchDoiRecord call
can throw for 404s and currently bubbles up as a server error; wrap the DOI
lookup so 404s map to notFound() while other errors still propagate: call
fetchDoiRecord(doi_id) inside a try/catch (or run Promise.allSettled for the
three calls and inspect the fetchDoiRecord result) and if the caught error
indicates a 404 then return notFound(), otherwise rethrow the error; keep the
surrounding Promise.all usage for fetchEvents and fetchDoisRecords (or
reconstruct the Promise.all with the resolved fetchDoiRecord value) and
reference fetchDoiRecord, fetchEvents, fetchDoisRecords, Promise.all, and
notFound() to locate and change the logic.
| if (citationsOverTime.length > 0) { | ||
| const yearNums = citationsOverTime.map((item: { year: string }) => | ||
| parseInt(item.year, 10), | ||
| ); | ||
| const minYear = Math.min(...yearNums); | ||
| const maxYear = new Date().getFullYear(); | ||
| const yearToCount: Record<string, number> = {}; | ||
| citationsOverTime.forEach((item: { year: string; total: number }) => { | ||
| yearToCount[item.year] = item.total; | ||
| }); | ||
| for (let y = minYear; y <= maxYear; y++) { | ||
| chartData.push({ | ||
| year: y.toString(), | ||
| count: yearToCount[y.toString()] ?? 0, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Filter invalid citation years before calling Math.min.
If any citationsOverTime entry contains a non-numeric year, minYear becomes NaN and the loop silently produces no chart rows.
Suggested fix
let chartData: { year: string; count: number }[] = [];
if (citationsOverTime.length > 0) {
- const yearNums = citationsOverTime.map((item: { year: string }) =>
- parseInt(item.year, 10),
- );
- const minYear = Math.min(...yearNums);
- const maxYear = new Date().getFullYear();
- const yearToCount: Record<string, number> = {};
- citationsOverTime.forEach((item: { year: string; total: number }) => {
- yearToCount[item.year] = item.total;
- });
- for (let y = minYear; y <= maxYear; y++) {
- chartData.push({
- year: y.toString(),
- count: yearToCount[y.toString()] ?? 0,
- });
+ const yearNums = citationsOverTime
+ .map((item: { year: string }) => Number.parseInt(item.year, 10))
+ .filter((year) => Number.isFinite(year));
+ if (yearNums.length > 0) {
+ const minYear = Math.min(...yearNums);
+ const maxYear = new Date().getFullYear();
+ const yearToCount: Record<string, number> = {};
+ citationsOverTime.forEach((item: { year: string; total: number }) => {
+ yearToCount[item.year] = item.total;
+ });
+ for (let y = minYear; y <= maxYear; y++) {
+ chartData.push({
+ year: y.toString(),
+ count: yearToCount[y.toString()] ?? 0,
+ });
+ }
}
}📝 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.
| if (citationsOverTime.length > 0) { | |
| const yearNums = citationsOverTime.map((item: { year: string }) => | |
| parseInt(item.year, 10), | |
| ); | |
| const minYear = Math.min(...yearNums); | |
| const maxYear = new Date().getFullYear(); | |
| const yearToCount: Record<string, number> = {}; | |
| citationsOverTime.forEach((item: { year: string; total: number }) => { | |
| yearToCount[item.year] = item.total; | |
| }); | |
| for (let y = minYear; y <= maxYear; y++) { | |
| chartData.push({ | |
| year: y.toString(), | |
| count: yearToCount[y.toString()] ?? 0, | |
| }); | |
| } | |
| } | |
| if (citationsOverTime.length > 0) { | |
| const yearNums = citationsOverTime | |
| .map((item: { year: string }) => Number.parseInt(item.year, 10)) | |
| .filter((year) => Number.isFinite(year)); | |
| if (yearNums.length > 0) { | |
| const minYear = Math.min(...yearNums); | |
| const maxYear = new Date().getFullYear(); | |
| const yearToCount: Record<string, number> = {}; | |
| citationsOverTime.forEach((item: { year: string; total: number }) => { | |
| yearToCount[item.year] = item.total; | |
| }); | |
| for (let y = minYear; y <= maxYear; y++) { | |
| chartData.push({ | |
| year: y.toString(), | |
| count: yearToCount[y.toString()] ?? 0, | |
| }); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/dois/`[...doi]/page.tsx around lines 41 - 57, Filter and validate
numeric years before computing minYear: transform citationsOverTime into numeric
years using parseInt (as you already do for yearNums), then remove NaN entries
(e.g., filter yearNums for Number.isFinite or !Number.isNaN) and only call
Math.min on the filtered array; if the filtered array is empty, skip building
the year loop or set a sensible fallback; update the code that produces
yearNums, minYear, and the for loop that fills chartData to use this validated
set so non-numeric years no longer produce NaN and the chart rows are generated
correctly.
1 participant
Purpose
closes: Add github issue that originated this PR
Approach
Open Questions and Pre-Merge TODOs
Learning
Types of changes
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Reviewer, please remember our guidelines:
Summary by CodeRabbit