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

[PCC-1371] Fix preview and publish to local targets #270

Merged
merged 18 commits into from
Jun 5, 2024

Conversation

a11rew
Copy link
Collaborator

@a11rew a11rew commented May 31, 2024

Overview

#269 fixed a regression in core causing preview and publish to not work with local (localhost://) targets.

This PR builds on that hotfix by removing the problematic code path that made the issue even possible and adds tests to prevent this from happening again.

Changes

core

  • Removed base url construction for redirects. All redirects are now to relative paths, which is valid in the HTTP spec. Trying to determine the request's absolute url in PantheonAPI is the problematic code path talked about above, it introduces unnecessary complexity we don't have to deal with when we just leave that to the HTTP client.
  • Removed app router specific code from core, essentially reverting it to its state in f5e47b3. This was to make it easy to validate the core PantheonAPI routing logic in tests without dealing with brower/Next.js constructs like Response and Headers
  • Added tests to validate routing logic

react-sdk

  • Moved changes added to core to support app router here
  • Added tests to validate the NextPantheonAPI here correctly handles requests and responses in both pages and app routing

vue-sdk

  • Removed app router support specific changes added here

Other changes

  • Updated build process in packages so sourcemaps are output separately instead of inline with application code which unnecessarily increases our package sizes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merged this into the core tests as it tested routing logic specific to PantheonAPI in core

@a11rew a11rew changed the title feat: decouple app router specific impl from core [PCC-1371] Fix preview and publish to local targets Jun 3, 2024
@a11rew a11rew marked this pull request as ready for review June 3, 2024 18:24
@a11rew a11rew requested review from kevinstubbs and aumkar June 3, 2024 18:24
@a11rew a11rew marked this pull request as draft June 3, 2024 21:25
@a11rew a11rew marked this pull request as ready for review June 3, 2024 21:26
@kevinstubbs
Copy link
Collaborator

Add changeset please.

@a11rew a11rew merged commit f9e4eee into main Jun 5, 2024
6 checks passed
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.

3 participants