Skip to content

Convert staging to app router [#1067]#1131

Merged
genehack merged 8 commits intomasterfrom
convert-staging-to-app-router-1067
Mar 24, 2025
Merged

Convert staging to app router [#1067]#1131
genehack merged 8 commits intomasterfrom
convert-staging-to-app-router-1067

Conversation

@genehack
Copy link
Contributor

@genehack genehack commented Mar 17, 2025

Description of proposed changes

Converts /staging over to the App Router style; see individual commit comments for details.

Preview — note that the error message for a bad dataset looks slightly different, due to how the base App Router layout is done. This feels like an acceptable modification.

Related issue(s)

Closes #1067

Checklist

@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-convert-st-d0asv2 March 17, 2025 20:48 Inactive
…1067]

Note: this will leave the site in a broken state; doing this to ensure
better Git tracking of file renames.
@genehack genehack force-pushed the convert-staging-to-app-router-1067 branch from a0120e2 to 229739a Compare March 17, 2025 20:49
@genehack genehack temporarily deployed to nextstrain-s-convert-st-d0asv2 March 17, 2025 20:49 Inactive
@victorlin victorlin self-requested a review March 17, 2025 23:15
@genehack genehack force-pushed the convert-staging-to-app-router-1067 branch from 229739a to 53b174c Compare March 20, 2025 22:48
@genehack genehack temporarily deployed to nextstrain-s-convert-st-d0asv2 March 20, 2025 22:48 Inactive
@genehack genehack force-pushed the convert-staging-to-app-router-1067 branch from 53b174c to bf10ac9 Compare March 20, 2025 22:53
@genehack genehack temporarily deployed to nextstrain-s-convert-st-d0asv2 March 20, 2025 22:53 Inactive
@genehack genehack force-pushed the convert-staging-to-app-router-1067 branch from bf10ac9 to 67088fd Compare March 21, 2025 03:13
@genehack genehack temporarily deployed to nextstrain-s-convert-st-d0asv2 March 21, 2025 03:14 Inactive
@genehack genehack force-pushed the convert-staging-to-app-router-1067 branch from 67088fd to ace67be Compare March 21, 2025 03:27
@genehack genehack temporarily deployed to nextstrain-s-convert-st-d0asv2 March 21, 2025 03:27 Inactive
@genehack
Copy link
Contributor Author

Okay, @victorlin, I think I addressed all the issues you raised; particularly interested in any feedback on the comment about how /staging versus /staging/foo works now.

I will plan to merge this early next week, sans additional feedback.

Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

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

Didn't test anything but the code looks good. Only comments are a non-blocking discussion for naming and a small typo correction.

* Split the callback out into a distinct React Client Component, to
  enable passing it to the <ListResources> component
* Split the page content out into a distinct React Server Component,
  so that it can be used in both the staging page.tsx file _and_ the
  staging not-found.tsx file
* Split the error banner handling out into a distinct React Client
  Component, so it can call `useParams()`
* Add a custom  not-found.tsx` component to allow displaying an error
  message when a bogus URL is requested, in addition to displaying the
  default staging page content
* Add Metadata API to the staging page
* Add a 'validateStagingUrl' "component", which detects when a bogus
  URL (e.g., /staging/foo) is requested, and calls the Next.js
  notFound() method so that the request renders the `not-found`
  component and sets a 404 status code on the response

NOTE: as of this commit, the site will be broken; there are additional
changes to the <ListResources> component that are required for this
code to work.
…ram [#1067]

This walks back a change that was made when converting the /pathogens
page to App Router syntax, moving back to an approach more similar to
the previous code, where a callback to list resources is passed into
this component.
#1067]

This switches the pathogens page back to passing a callback to the
<ListResources> component, rather than having that component directly
import the callback itself.
Currently, trying to load something like `/pathogens/foo` results in a
404 error.

If we _were_ to add display of an error banner instead (the way we do
for `/staging/foo`), using a fully-specified URL will be required to
still be able to load the normal `/pathogens` data under the error
message.
@genehack genehack force-pushed the convert-staging-to-app-router-1067 branch from ace67be to b281971 Compare March 24, 2025 19:44
@genehack genehack temporarily deployed to nextstrain-s-convert-st-d0asv2 March 24, 2025 19:45 Inactive
@genehack genehack merged commit 15d41e9 into master Mar 24, 2025
8 checks passed
@genehack genehack deleted the convert-staging-to-app-router-1067 branch March 24, 2025 19:53
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.

Convert /staging/[[...staging]].jsx to App Router

4 participants