-
-
Notifications
You must be signed in to change notification settings - Fork 263
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(router): provide server context awareness to routing and HttpClient requests #1223
Conversation
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for analog-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for analog-ng-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
return baseUrl; | ||
} | ||
|
||
export function getRequestProtocol( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi, I took this function from here 😄 https://github.com/unjs/h3/blob/bd7831bde071d368cf3f120128cc1823bfb4c3c8/src/utils/request.ts#L238
But I implemented it in the code because Initially, I hadn't access to the H3Event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In development, we won't have access to the H3Event
, but the req
and res
are the same with the Vite server in development, and Nitro in the production server.
export function requestContextInterceptor( | ||
req: HttpRequest<unknown>, | ||
next: HttpHandlerFn | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Qq, is this interceptor only for fetching data before loading a page, right?
In the analog app, I have implemented some API routes, and I made a request from an angular service using the HttpClient, and I didn't have to use an interceptor to prepend the full URL; the behavior I assumed was that as long as the angular app has the correct base URL, I don't have to worry about how the URLs are handled in ssr and csr, all work at the same.
I haven't commented before because I haven't tried the SSR data fetching so far https://analogjs.org/docs/features/data-fetching/server-side-data-fetching
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's for any request with HttpClient that begins with a /
meaning its meant to be an "internal" request.
By default, even if the base href is set in the index.html
, it won't affect HttpClient requests.
On the server, the full URL is used
One the client, we can use the full URL or just leave it as is
During pre-rendering, API requests remain as-is. Any other requests. Use the full URL.
I'm thinking of including support for passing some header to skip this interceptor also.
Preview deployments tested and working with Node, Netlify, Cloudflare, and Vercel. |
i wonder if the provider function name shouldn't it be |
It intentionally begins with |
I guess, I couldn't express myself 😅 I starts with |
Ahhh, great catch. Yea, I've been staring at it too long 😅 |
PR Checklist
Closes #844
Closes #995
Closes #1182
What is the new behavior?
provideServerContext
provider function from@analogjs/router/server
injectBaseURL()
,injectRequest()
, andinjectResponse()
functions from@analogjs/router/tokens
requestContextInterceptor
function that is environment aware when making HttpClient requestsBy providing the server context, and request interceptor. Any HttpClient request that begins with a
/
is requested using the full URL on the server, client, and during prerendering without manually setting theVITE_ANALOG_PUBLIC_BASE_URL
environment variable.credit goes to @osnoser1 for the initial implementation
An example service that fetches todos from the API endpoint.
Does this PR introduce a breaking change?
Other information
[optional] What gif best describes this PR or how it makes you feel?