-
-
Notifications
You must be signed in to change notification settings - Fork 226
-
-
Notifications
You must be signed in to change notification settings - Fork 226
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
From Discord: Discussion on Correct Usage of URL Parsing in ServerRequest and Naming Conventions #3639
Comments
REPRO: import {
HttpMiddleware,
HttpServer,
HttpServerRequest,
HttpServerResponse,
} from "@effect/platform";
import { NodeHttpServer, NodeRuntime } from "@effect/platform-node";
import { Layer, Effect } from "effect";
import { createServer } from "node:http";
const ServerLive = NodeHttpServer.layer(() => createServer(), { port: 3000 });
const HttpLive = HttpServer.serve(
Effect.gen(function* () {
const req = yield* HttpServerRequest.HttpServerRequest;
console.log(req.url);
console.log(req.originalUrl);
const searchParams = yield* HttpServerRequest.ParsedSearchParams;
console.log(searchParams);
return HttpServerResponse.text("hi");
}).pipe(HttpMiddleware.searchParamsParser, HttpMiddleware.logger)
).pipe(HttpServer.withLogAddress, Layer.provide(ServerLive));
NodeRuntime.runMain(Layer.launch(HttpLive));
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
The user is discussing a potential issue in the Effect-TS codebase, specifically in the
httpMiddleware.ts
file. They suggest that the code should useServerRequest.searchParamsFromURL(new URL(request.originalUrl))
instead of the current implementation becauseServerRequest.url
is stripped.Additionally, the user questions the naming conventions used in the
ServerRequest
object, particularly the distinction betweenServerRequest.url
andServerRequest.originalUrl
. They express concern that deviating from the web standardRequest
'surl
property might be confusing and suggest using a different term for the parsed URL, similar to how Next.js usesNextRequest.pathname
.Key Takeaways:
ServerRequest.url
vs.ServerRequest.originalUrl
.Discord thread
https://discord.com/channels/795981131316985866/1286042509616746519
The text was updated successfully, but these errors were encountered: