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

feat: add new redirects feature #2583

Merged
merged 9 commits into from
Dec 16, 2024
Merged

Conversation

pedromtec
Copy link
Contributor

@pedromtec pedromtec commented Dec 4, 2024

What's the purpose of this pull request?

The implementation of the redirects feature in FastStore appears as a strategic solution to facilitate the migration of old stores to our platform.

The idea of ​​this feature is to run the redirect in the getStaticProps function when a certain page is not found. In other words, before rendering a not found (404) result to the user, we check if there is a redirect associated with the pathname requested by the user.

Since the pathname is not found (404). The redirect flow follows two steps. First it goes through a customization function called matcher. If the matcher cannot resolve the redirects, we validate whether a redirect exists in the rewriter database.

Captura de Tela 2024-12-10 às 10 18 24

Components

  • Matcher: Optional function that can be implemented via customization. Some pathnames can be resolved via simple logic, without necessarily going to a database to check if there is a redirect registered. See an example.

  • Rewriter: If the redirect is not resolved via customization, a call will be made to the rewriter api to check if there is a redirect associated with the requested pathname.

Redirects RFC

How to test it?

https://storeframework.myvtex.com/admin/cms/redirects

Captura de Tela 2024-12-12 às 10 08 49

We can run locally and try to access one of these redirects.

Example of matcher in starter: vtex-sites/starter.store#626

Example: /produto/1

Copy link

vercel bot commented Dec 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
faststore-site ⬜️ Ignored (Inspect) Visit Preview Dec 16, 2024 7:52pm

Copy link

codesandbox-ci bot commented Dec 4, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@eduardoformiga eduardoformiga force-pushed the feat/redirect-in-get-static-props branch from 3ae0eaf to 86ce7da Compare December 11, 2024 20:15
@pedromtec pedromtec changed the title feat: redirect in getStaticProps feat: add new redirects feature Dec 12, 2024
@pedromtec pedromtec marked this pull request as ready for review December 12, 2024 13:41
@pedromtec pedromtec requested a review from a team as a code owner December 12, 2024 13:41
@pedromtec pedromtec requested review from hellofanny and emersonlaurentino and removed request for a team December 12, 2024 13:41
@pedromtec pedromtec force-pushed the feat/redirect-in-get-static-props branch from bedf1e0 to 017fc1f Compare December 12, 2024 20:45
Copy link
Contributor

@lariciamota lariciamota left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Tested using the starter preview with the matcher and also tested the core adding enableRedirects true, it worked as expected!

}

const response = await fetch(
`https://${storeConfig.api.storeId}.myvtex.com/redirect-evaluate${pathname}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to change this endpoint.

@pedromtec pedromtec merged commit 4624a5d into main Dec 16, 2024
8 checks passed
@pedromtec pedromtec deleted the feat/redirect-in-get-static-props branch December 16, 2024 20:33
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