fix: make interceptors work in any order even when filteringScope

[salmanm]

Fixed #1818

[salmanm]

@paulmelnikow Wanna review this PR? Or invite someone who would?

[mastermatt]

@salmanm thanks for taking this on.

The interceptorsFor function is one I'd really like to get rid of. I've been working on it in other refactors, but I have no timeline for that so we should move forward with this change.

As noted in the original issue, the bug is that the function returns early before it has a chance to set __nock_filteredScope on all the Interceptors.
Your change fixes that, but it means the filteringScope function has to be run for each Interceptor in the array. I always like to assume that user input function can be expensive.

My suggestion is to replace lines 153-169 with this:

    const { filteringScope } = interceptors[0].scope.scopeOptions

    // If scope filtering function is defined and returns a truthy value then
    // we have to treat this as a match.
    if (filteringScope && filteringScope(basePath)) {
      debug('found matching scope interceptor')

      // Keep the filtered scope (its key) to signal the rest of the module
      // that this wasn't an exact but filtered match.
      interceptors.forEach(ic => {
        ic.__nock_filteredScope = key
      })
      return interceptors
    }

Things to note: interceptors is guaranteed to always have at least one item, and all the interceptors in the array are guaranteed to have the same scope.
.__nock_scopeOptions is a ref to .scope.scopeOptions and .__nock_scopeKey is a copy of key.
I think we should move away from these magic dunder attributes.

[salmanm]

Ok those two clarifications changes things. I didn't know that they're guaranteed to have the same scope. I'll change it to you suggestion of a sub loop to set the __nock_filteredScope.

and yes, I agree we should get rid of those magic attribs, but perhaps that's a bigger refactor and can be done separate from this PR?

[salmanm]

Aye pushed the suggested changes. 👍

[salmanm]

@mastermatt Would you like to take another look?

[mastermatt]

Works for me.

@paulmelnikow ?

[salmanm]

Can we move this forward please? Or let me know if there are any changes. Thanks @paulmelnikow.

[nockbot]

🎉 This PR is included in version 11.7.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀