Skip to content

feat(i18n): apply specific routing logic only to pages#9091

Merged
ematipico merged 3 commits intomainfrom
feat/apply-routing-to-pages-only
Nov 15, 2023
Merged

feat(i18n): apply specific routing logic only to pages#9091
ematipico merged 3 commits intomainfrom
feat/apply-routing-to-pages-only

Conversation

@ematipico
Copy link
Copy Markdown
Member

@ematipico ematipico commented Nov 13, 2023

Changes

This PR addresses this new part of the RFC:withastro/roadmap@c93015d

There have been reports where:

  • the assets endpoint doesn't work properly
  • users need to have services that aren't part of the routing logic
  • the RSS integration stopped working as expected

After chatting with Matthew, we were more convinced that i18n routing should only work on pages. This PR updates the logic by.

I created some new logic inside the Pipeline, called hooks. Essentially, I created a generic way to run functions before before rendering a route. The reason why I created this generic logic is because I wanted to have a generic function inside my middleware, and then share the same logic among pipelines (dev, SSR and SSG).

This worked out pretty well and I am satisfied with the result.

Testing

Added:

  • a case for assets
  • a new case with an endpoint that should render

Docs

N/A but I will make a PR to reflect the change in logic

@ematipico ematipico force-pushed the feat/apply-routing-to-pages-only branch from c79944e to 56d611f Compare November 13, 2023 20:23
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 13, 2023

🦋 Changeset detected

Latest commit: 5083580

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Nov 13, 2023
@ematipico ematipico force-pushed the feat/apply-routing-to-pages-only branch 2 times, most recently from 86a3e29 to eb2789d Compare November 15, 2023 16:35
@ematipico ematipico force-pushed the feat/apply-routing-to-pages-only branch from eb2789d to 5083580 Compare November 15, 2023 16:47
@ematipico ematipico requested a review from matthewp November 15, 2023 17:41
Copy link
Copy Markdown
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

lgtm!

@ematipico ematipico merged commit 536c6c9 into main Nov 15, 2023
@ematipico ematipico deleted the feat/apply-routing-to-pages-only branch November 15, 2023 18:43
This was referenced Nov 15, 2023
peng added a commit to peng/astro that referenced this pull request Nov 17, 2023
* main:
  feat(i18n): add `Astro.currentLocale` (withastro#9101)
  [ci] release (withastro#9107)
  Add compatibility with cloudflare node (withastro#8925)
  [ci] format
  Cancel response stream when connection closes (withastro#9071)
  [ci] format
  feat(i18n): apply specific routing logic only to pages (withastro#9091)
  feat(dev-overlay): Hide plugins into a separate menu when there's too many enabled (withastro#9102)
  [ci] format
  Support Svelte 5 (experimental) (withastro#9098)
  [ci] release (withastro#9078)
  [ci] format
  Refactor shikiji syntax highlighting code (withastro#9083)
  [ci] format
  fix: Query params trigger the trailingSlash error in preview mode (withastro#9045)
  fix(assets): bundling regression for specific config on non-Node runtimes (withastro#9087)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants