Skip to content
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: Fixes reroute behavior for PR previews #345

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

ShrimpCryptid
Copy link
Contributor

@ShrimpCryptid ShrimpCryptid commented Dec 17, 2024

Problem

Noticed on one of my other PRs that the GH reroute behavior wasn't working correctly in the PR preview. If you refreshed while you were in the /viewer page, it would give you a 404 error. It turns out I forgot to copy some of the code logic from TFE that makes PR previews work.

This change copies code from TFE, so I'll include links to the original code for reference.

Estimated review size: small, 10 minutes

Solution

  • Fixes PR preview redirects, so opening links with query parameters or reloading preview pages will work correctly.
    • Adds checks for PR previews during 404 redirects.
    • Renames several methods in gh_route_utils.ts.
    • Updates URL parsing logic in index.tsx.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Testing and validation

I manually triggered a nightly build from this branch and confirmed that you can now refresh from the PR preview pages without getting a 404 error. This change is temporary so after today (12/24) the PR previews will be broken again until this change is merged.

Video talk-through (🔊):

2024-12-24.12-20-24.mp4

@ShrimpCryptid ShrimpCryptid added the bug Something isn't working label Dec 17, 2024
@ShrimpCryptid ShrimpCryptid self-assigned this Dec 17, 2024
Copy link

github-actions bot commented Dec 17, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2025-01-13 22:05 UTC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ShrimpCryptid ShrimpCryptid marked this pull request as ready for review December 24, 2024 20:28
@ShrimpCryptid ShrimpCryptid requested a review from a team as a code owner December 24, 2024 20:28
@ShrimpCryptid ShrimpCryptid requested review from meganrm and ascibisz and removed request for a team December 24, 2024 20:28
@ShrimpCryptid ShrimpCryptid merged commit 1a32331 into main Jan 13, 2025
4 checks passed
@ShrimpCryptid ShrimpCryptid deleted the fix/pr-preview-redirects branch January 13, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants