fix: Agent.Options.factory should accept URL object or string as parameter

[nicole0707]

This relates to...

Based on the below declaration, the first parameter of Agent.Options.factory should be URL Object.

declare namespace Agent {
  export interface Options extends Pool.Options {
    /** Default: `(origin, opts) => new Pool(origin, opts)`. */
    factory?(origin: URL, opts: Object): Dispatcher;
    /** Integer. Default: `0` */
    maxRedirections?: number;
    interceptors?: { Agent?: readonly Dispatcher.DispatchInterceptor[] } & Pool.Options["interceptors"]
  }
}

However, it's being passed in string or URL object now.

  [kDispatch] (opts, handler) {
    let key
    if (opts.origin && (typeof opts.origin === 'string' || opts.origin instanceof URL)) {
      key = String(opts.origin)
    } else {
      throw new InvalidArgumentError('opts.origin must be a non-empty string or URL.')
    }

Rationale

Changes

Features

Bug Fixes

Update the definition of Agent, Agent.Options.factory has supported string | URL type now.

      dispatcher = this[kFactory](new URL(opts.origin), this[kOptions])

Breaking Changes and Deprecations

Status

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

[nicole0707]

@Ethan-Arrowood Are you free to review my PR? If anything is missing in this PR, please let me know. The discrepancy has caused an issue in our product, cheers.

[ronag]

What problem does this solve? Looks more like the type definitions are wrong.

[nicole0707]

What problem does this solve? Looks more like the type definitions are wrong.

Yes, definition is inconsistent with implementation. Do you prefer updating definition to factory?(origin: string | URL, opts: Object): Dispatcher;? I just check Client and Pool constructor definition, they both allow string | URL.

[nicole0707]

Hi @ronag , I've updated the definition and test as suggested, cheers!

[mcollina]

lgtm