Skip to content
This repository has been archived by the owner on May 5, 2023. It is now read-only.

Syncing patched request's method signature with https://nodejs.org/… #30

Closed
wants to merge 1 commit into from

Conversation

emersonford
Copy link

…api/https.html#https_https_get_options_callback. The argument checking is significantly more robust with unit tests checking every permutation of arguments. This solves issue #29

@emersonford
Copy link
Author

It looks like it's failing because the spread args aren't supported until nodejs 5.0. nodejs <6.0 has been EOL'd so it might be worth dropping support for those older versions?

TooTallNate added a commit that referenced this pull request Dec 2, 2019
Instead of patching over `https.request()` and `https.get()` manually
to add the `secureEndpoint` boolean, utilize the stack trace of the
current call logic to determine whether or not the `https.js` core
module is invoking `new ClientRequest()`.

In theory, this is still fragile logic, but should be more future-proof
and cleaner than patching over a core module, which is never really
desirable.

Fixes #29.
Closes #30.
Closes #35.
TooTallNate added a commit that referenced this pull request Dec 6, 2019
* Remove `https` core module patching logic

Instead of patching over `https.request()` and `https.get()` manually
to add the `secureEndpoint` boolean, utilize the stack trace of the
current call logic to determine whether or not the `https.js` core
module is invoking `new ClientRequest()`.

In theory, this is still fragile logic, but should be more future-proof
and cleaner than patching over a core module, which is never really
desirable.

Fixes #29.
Closes #30.
Closes #35.

* Remove Node 10-specific test case

* Prettier
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant