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

Export CORS constants for convenience #526

Merged
merged 2 commits into from
Mar 13, 2023
Merged

Conversation

timostamm
Copy link
Member

This adds the export cors to @bufbuild/connect. This object provides helpful constants to configure CORS middleware for cross-domain requests with the protocols supported by Connect.

This adds the export `cors` to @bufbuild/connect. This object provides helpful constants to configure CORS middleware for cross-domain requests with the protocols supported by Connect.
@fubhy
Copy link
Contributor

fubhy commented Mar 13, 2023

Timo, wouldn't an approach around or similiar to interceptors on the ConnectRouter be a good place for handling CORS too? There are use-cases for CORS where you want to explicitly enable it for some service methods but not for others. Hence, tying it to the router seems reasonable instead of lifting it to the server / adapter level.

I mentioned that also in #527

@timostamm
Copy link
Member Author

Timo, wouldn't an approach around or similiar to interceptors on the ConnectRouter be a good place for handling CORS too?

I think Connect routes should receive only requests in protocol. This export will make it really easy to configure @fastify/cors and the like properly. It's just a small incremental improvement.

@fubhy
Copy link
Contributor

fubhy commented Mar 13, 2023

Timo, wouldn't an approach around or similiar to interceptors on the ConnectRouter be a good place for handling CORS too?

I think Connect routes should receive only requests in protocol. This export will make it really easy to configure @fastify/cors and the like properly. It's just a small incremental improvement.

OK. The only limitation there would be that you wouldn't have the gRPC service method context outside of the router. But I can see wour point.

@timostamm timostamm merged commit 7564182 into main Mar 13, 2023
@timostamm timostamm deleted the tstamm/add-cors-constants branch March 13, 2023 17:17
This was referenced Mar 15, 2023
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