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

Relax type of callback arguments to Router methods #605

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

compiler-errors
Copy link
Contributor

This PR relaxes the type of the callback

func: fn(Request, RouteContext<D>) -> T

into

func: impl Fn(Request, RouteContext<D>) -> T + 'static

which is an argument-position impl trait type (APIT), since I don't see any reason for requiring the passed-in callback is a function pointer, since we're immediately boxing it up and erasing the type.

Specifically, this ensures that you can pass a (currently nightly-only) async closure (async ||), which currently does not support being coerced to a fn ptr type. I'd like to avoid needing to implement that functionality that if possible.

Comment on lines 193 to 198
pub fn head_async<T>(
mut self,
pattern: &str,
func: impl Fn(Request, RouteContext<D>) -> T + 'static,
) -> Self
where
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, this doesn't need to be an APIT, but could also just be a generic parameter.

Suggested change
pub fn head_async<T>(
mut self,
pattern: &str,
func: impl Fn(Request, RouteContext<D>) -> T + 'static,
) -> Self
where
pub fn head_async<T>(
mut self,
pattern: &str,
func: F,
) -> Self
where
F: Fn(Request, RouteContext<D>) -> T + 'static,

pub fn head_async<T>(
mut self,
pattern: &str,
func: impl Fn(Request, RouteContext<D>) -> T + 'static,
Copy link
Member

Choose a reason for hiding this comment

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

I guess my only concern is if the 'static bound will break existing code in any circumstance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't matter for closures or fn item types but would matter for fn pointer types, but also I missed that we really only need to restrict this to + 'a.

@kflansburg kflansburg merged commit 51e1feb into cloudflare:main Aug 27, 2024
3 checks passed
@compiler-errors compiler-errors deleted the impl-Fn branch August 27, 2024 13:28
thibmeu pushed a commit to thibmeu/workers-rs that referenced this pull request Sep 2, 2024
renovate bot added a commit to spiraldb/vortex that referenced this pull request Sep 13, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [worker](https://redirect.github.com/cloudflare/workers-rs) |
workspace.dependencies | minor | `0.3.0` -> `0.4.0` |

---

### Release Notes

<details>
<summary>cloudflare/workers-rs (worker)</summary>

###
[`v0.4.0`](https://redirect.github.com/cloudflare/workers-rs/releases/tag/v0.4.0)

[Compare
Source](https://redirect.github.com/cloudflare/workers-rs/compare/v0.3.4...v0.4.0)

#### What's Changed

- Relax type of callback arguments to `Router` methods. This uses APIT
to allow more types than a function pointer to implement handlers (i.e.
async closure) by
[@&#8203;compiler-errors](https://redirect.github.com/compiler-errors)
in
[cloudflare/workers-rs#605
- Fix typos in CORS argument names by
[@&#8203;OliverEvans96](https://redirect.github.com/OliverEvans96) in
[cloudflare/workers-rs#629
- Implement `get_all` function to return non-folding set-cookie headers
by [@&#8203;nakamura-shuta](https://redirect.github.com/nakamura-shuta)
in
[cloudflare/workers-rs#597
- Add `FormData` conversion into `JsValue` by
[@&#8203;thibmeu](https://redirect.github.com/thibmeu) in
[cloudflare/workers-rs#634

> \[!CAUTION]
> Breaking: Make R2 `Object::size` return `u64` by
[@&#8203;lkolbly](https://redirect.github.com/lkolbly) in
[cloudflare/workers-rs#625

#### New Contributors

- [@&#8203;lkolbly](https://redirect.github.com/lkolbly) made their
first contribution in
[cloudflare/workers-rs#625
- [@&#8203;compiler-errors](https://redirect.github.com/compiler-errors)
made their first contribution in
[cloudflare/workers-rs#605
- [@&#8203;OliverEvans96](https://redirect.github.com/OliverEvans96)
made their first contribution in
[cloudflare/workers-rs#629
- [@&#8203;nakamura-shuta](https://redirect.github.com/nakamura-shuta)
made their first contribution in
[cloudflare/workers-rs#597
- [@&#8203;thibmeu](https://redirect.github.com/thibmeu) made their
first contribution in
[cloudflare/workers-rs#634

**Full Changelog**:
cloudflare/workers-rs@v0.3.4...v0.4.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/spiraldb/vortex).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC43NC4xIiwidXBkYXRlZEluVmVyIjoiMzguNzQuMSIsInRhcmdldEJyYW5jaCI6ImRldmVsb3AiLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

2 participants