fix: Improve Dispatcher.compose when nullish values are passed as argument

[matthieusieben]

This relates to...

N/A

Rationale

The implementation of Dispatcher.compose allows nullish values:

undici/lib/dispatcher/dispatcher.js

Lines 23 to 25 in ae3efd4

	if (interceptor == null) {
	continue
	}

This change alligns the type definitons with this to allow inline definitions:

dispatcher.compose(
  maxRedirections > 0
    ? interceptor.redirect({ maxRedirections })
    : null
)

Changes

Just updated the type definition of the arguments of Dispatcher.compose from Dispatcher.DispatcherComposeInterceptor[] to (null | undefined | Dispatcher.DispatcherComposeInterceptor)[]

Features

N/A

Bug Fixes

N/A

Breaking Changes and Deprecations

N/A

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (56bb5a0) to head (7a2ab7e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5443      +/-   ##
==========================================
+ Coverage   93.44%   93.47%   +0.02%     
==========================================
  Files         110      110              
  Lines       37078    37108      +30     
==========================================
+ Hits        34649    34688      +39     
+ Misses       2429     2420       -9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mcollina]

can you update the type tests?

[matthieusieben]

I've added tests, and also changed the implementation a bit to avoid returning a proxy object if no interceptor are actually provided

[metcoder95]

(guilty of implementing that) I believe is better to just not accept nullish values at all, it does not provides any value and now makes it look a bit odd as anyways the null values do not do anything at all.

Happy to keep this PR as is to avoid breaking changes, but maybe set a warning that preempts this behaviour will change in further major.

[matthieusieben]

It does provide some value when creating the dispatcher as it allows doing:

return dispatcher.compose(
  maxRedirections > 0
    ? interceptor.redirect({ maxRedirections })
    : null,
  maxRetries > 0
    ? interceptor.retry({ ... })
    : null
)

instead of:

import { DispatcherComposeInterceptor } from 'undici'

const interceptors: DispatcherComposeInterceptor[] = []

if (maxRedirections > 0) {
  interceptors.push(interceptor.redirect({ maxRedirections }))
}

if (maxRetries > 0) {
  interceptors.push(interceptor.retry({...}))
}

return dispatcher.compose(interceptors)

I find the latter too verbose, and is also requires to explicitly import Interceptor as a type.

[metcoder95]

That's a good point, given your example, I believe then is more a DX thingy that something adding a runtime value. So I'm happy either way 👍