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

enableNetConnect() no longer works with Regexp-like objects #1861

Closed
nikaspran opened this issue Jan 30, 2020 · 3 comments · Fixed by #1889
Closed

enableNetConnect() no longer works with Regexp-like objects #1861

nikaspran opened this issue Jan 30, 2020 · 3 comments · Fixed by #1889

Comments

@nikaspran
Copy link
Contributor

nikaspran commented Jan 30, 2020

This is not necessarily a bug, but something I would consider an undocumented feature. Before https://github.com/nock/nock/pull/1754/files#diff-503754c07b5098227646f913eee85885R58 it was possible to use enableNetConnect with a function, which greatly improved the flexibility of this mechanism.

As an example use case, we used to have a constantly changing Set of allowed hostnames which tests could add to. This is still possible to achieve by constructing a RegExp but it is much more awkward. There may be other use cases which cannot be converted to a RegExp at all.

What is the expected behavior?

This used to work:

const allowedOutsideHosts = [/* some hosts */];
nock.enableNetConnect({
  test: host => allowedOutsideHosts.includes(host), // or any function
});

What is the actual behavior?

You can no longer use "RegExp-like" objects (with a test function) to filter hosts for enableNetConnect(). It now defaults to a wildcard match matching anything.

Possible solutions

  1. Revert back to checking for a test function?
  2. Allow passing an explicit object with a test function

How to reproduce the issue

Runkit: 11.7.0
Runkit: 11.7.2

Versions

Software Version(s)
Nock 11.7.1
Node Any

I would be willing to submit a PR for this over the next few days if you agree that it should be done. I personally think that option 2 might be the best as it would retain the proper typechecks.

Thank you!

@paulmelnikow
Copy link
Member

Hi, thanks for the report. This seems like a worthwhile feature. Though I think this is a feature request, since this behavior was not documented.

It'd probably be better implemented by passing a function to enableNetConnect(), rather than a regexpy duck. Also related to #1753, which is proposing some changes related to enableNetConnect.

@nikaspran
Copy link
Contributor Author

Fair enough - the muddy nature of bug vs feature is exactly why I didn't wanna go ahead with an implementation before consulting. I'll prepare a PR to add support for a function parameter.

@github-actions
Copy link

🎉 This issue has been resolved in version 11.9.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants