Skip to content

Convert export page to be CSR#876

Open
thostetler wants to merge 5 commits into
adsabs:masterfrom
thostetler:feat/export-ssr-strip
Open

Convert export page to be CSR#876
thostetler wants to merge 5 commits into
adsabs:masterfrom
thostetler:feat/export-ssr-strip

Conversation

@thostetler
Copy link
Copy Markdown
Member

Export page has been extremely slow to load, and since most of the wait is done server-side waiting on the API to respond we can offload that to the client to take advantage of the RC caching and being able to show skeletons.

This also removes the infinite style calls, which probably aren't necessary here.

Copilot AI review requested due to automatic review settings May 28, 2026 19:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.7%. Comparing base (16840c8) to head (4d55fb7).

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #876     +/-   ##
========================================
+ Coverage    62.6%   62.7%   +0.1%     
========================================
  Files         324     325      +1     
  Lines       38203   38184     -19     
  Branches     1733    1733             
========================================
+ Hits        23903   23920     +17     
+ Misses      14259   14223     -36     
  Partials       41      41             
Files with missing lines Coverage Δ
...c/components/CitationExporter/CitationExporter.tsx 85.8% <ø> (+9.0%) ⬆️
...nts/CitationExporter/components/ExportSkeleton.tsx 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/pages/search/exportcitation/[format].tsx:163

  • In the getServerSideProps error path, format is omitted from returned props even though the page expects it (and uses it to derive bibtex settings). Return a sensible format fallback (e.g., the URL param or ExportApiFormatKey.bibtex) in the catch block to keep rendering consistent.
  } catch (error) {
    logger.error({ msg: 'GSSP error in export citation page', error });
    return {
      props: {
        query: searchParams,
        pageError: parseAPIError(error),
        error: axios.isAxiosError(error) ? error.message : 'Unable to fetch data',
      },
    };

Comment on lines 26 to 34
@@ -35,10 +34,9 @@ interface IExportCitationPageProps {
}
Comment on lines 120 to 134
export const getServerSideProps: GetServerSideProps = composeNextGSSP(async (ctx) => {
const {
qid = null,
p,
referrer = null,
...query
} = parseQueryFromUrl<{ qid: string; format: string }>(ctx.req.url, { sortPostfix: 'id asc' });

const { format } = ctx.params as { format: string };

if (!query && !qid) {
return {
props: {
format,
query,
qid,
referrer,
error: 'No Records',
},
};
}

const queryClient = new QueryClient();
const params: IADSApiSearchParams = {
const searchParams: IADSApiSearchParams = {
rows: APP_DEFAULTS.EXPORT_PAGE_SIZE,
fl: ['bibcode'],
sort: query.sort ?? APP_DEFAULTS.SORT,
...(qid ? { q: `docs(${qid})` } : query),
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants