Remove automatic cross-compatibility for hooks/middleware#1316
Conversation
This one's related to #1284. In that pull request, we made it so that any hooks sent to `Config.Hooks` that implemented middleware or any middleware sent to `Config.Middleware` that implement hooks were automatically extracted and use for all types. I've been thinking about this for the last couple of days, and in retrospect, I think this automatic compatibility might be a little unnecessarily aggressive. The case in favor is that when adding additional hook-related functionality to something like `otelriver`, the user can just leave in their existing instance in middleware and not change anything to get the existing functionality. (Without the automatic compatibility they'd have to move it to `Config.Plugins` instead.) The case against is that it may result in some surprising behavior if you only intended to add something as either a hook or middleware. We haven't shipped a release containing #1284 yet, so we can still change this if we want to. I didn't ship a strict revert because #1284 also brings in `Config.Plugins` and some useful internal changes to support everything.
|
Thanks! Okay, I couldn't fully decide whether to merge this one or not (can make arguments both directions), so I sent it through Codex and asked it to be as objective as possible:
That works for me, so going to pull this in. |
This one's related to #1284. In that pull request, we made it so that
any hooks sent to
Config.Hooksthat implemented middleware or anymiddleware sent to
Config.Middlewarethat implement hooks wereautomatically extracted and use for all types.
I've been thinking about this for the last couple of days, and in
retrospect, I think this automatic compatibility might be a little
unnecessarily aggressive. The case in favor is that when adding
additional hook-related functionality to something like
otelriver, theuser can just leave in their existing instance in middleware and not
change anything to get the existing functionality. (Without the automatic
compatibility they'd have to move it to
Config.Pluginsinstead.) Thecase against is that it may result in some surprising behavior if you
only intended to add something as either a hook or middleware.
We haven't shipped a release containing #1284 yet, so we can still
change this if we want to. I didn't ship a strict revert because #1284
also brings in
Config.Pluginsand some useful internal changes tosupport everything.