Skip to content

Conversation

@cliffordfajardo
Copy link

@cliffordfajardo cliffordfajardo commented Sep 20, 2022

What is the change?

  • Add typings for https agent
  • Add correct typings fetch's options param

Why are we making this change?

Tests

Type changes only. Ran npm test inside of package/fetch and all tests pass

Test output for `package/fetch`
 247 passing (4s)
  4 pending

-----------------|---------|----------|---------|---------|-----------------------------------------
File             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                       
-----------------|---------|----------|---------|---------|-----------------------------------------
All files        |   97.15 |    96.79 |   89.32 |   97.15 |                                         
 src             |   96.69 |    96.58 |   86.58 |   96.69 |                                         
  body.js        |   96.97 |    95.45 |   97.36 |   96.97 | 211-212,250-254,263,464-473             
  fetch.js       |   99.46 |    96.38 |     100 |   99.46 | 117-118                                 
  headers.js     |   92.73 |    93.75 |   81.81 |   92.73 | 16-24,29-38,95-96                       
  lib.node.js    |     100 |      100 |     100 |     100 |                                         
  package.js     |     100 |      100 |     100 |     100 |                                         
  request.js     |   95.62 |      100 |   56.25 |   95.62 | ...-170,173-174,178-179,183-184,188-189 
  response.js    |   98.59 |      100 |    90.9 |   98.59 | 59-60                                   
 src/errors      |     100 |      100 |     100 |     100 |                                         
  abort-error.js |     100 |      100 |     100 |     100 |                                         
  base.js        |     100 |      100 |     100 |     100 |                                         
  fetch-error.js |     100 |      100 |     100 |     100 |                                         
 src/utils       |   99.27 |     97.4 |     100 |   99.27 |                                         
  form-data.js   |   98.29 |       92 |     100 |   98.29 | 115-116                                 
  get-search.js  |     100 |      100 |     100 |     100 |                                         
  is-redirect.js |     100 |      100 |     100 |     100 |                                         
  is.js          |     100 |      100 |     100 |     100 |                                         
  utf8.js        |     100 |      100 |     100 |     100 |                                         
-----------------|---------|----------|---------|---------|-----------------------------------------

@changeset-bot
Copy link

changeset-bot bot commented Sep 20, 2022

🦋 Changeset detected

Latest commit: a9da0a7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@remix-run/web-fetch Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cliffordfajardo
Copy link
Author

@mcansh - trying to add a change set but I only see the option to make a major bump in the CLI 🤔
Is that correct?

CleanShot 2022-09-20 at 13 54 00@2x

@mcansh
Copy link
Collaborator

mcansh commented Sep 20, 2022

@mcansh - trying to add a change set but I only see the option to make a major bump in the CLI 🤔

Is that correct?

CleanShot 2022-09-20 at 13 54 00@2x

if you hit enter without selecting a package, it should prompt for minor and then patch. however you can always change the changeset after :)

however, i usually just do it via the changeset bot prompt

@cliffordfajardo
Copy link
Author

cliffordfajardo commented Sep 20, 2022

Updated to patch in a following commit 😄
Hopefully github squash & merge should fix the unnecessary commit

EDIT: updating to minor

@cliffordfajardo cliffordfajardo force-pushed the cfajardo/add-https-agent-typings branch from 1319655 to a9da0a7 Compare September 20, 2022 21:09
@cliffordfajardo
Copy link
Author

Updated

@cliffordfajardo
Copy link
Author

@mcansh - any other changes you think I should make?

@mcansh
Copy link
Collaborator

mcansh commented Sep 26, 2022

@mcansh - any other changes you think I should make?

sorry @cliffordfajardo i was out the last 2 weeks, but was popping in and out of GitHub, I'll get this reviewed shortly :)

Copy link
Collaborator

@mcansh mcansh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, gonna get another approval real quick before merging :)

@jacob-ebey jacob-ebey merged commit 6d9cd44 into remix-run:main Sep 26, 2022
@cliffordfajardo cliffordfajardo deleted the cfajardo/add-https-agent-typings branch September 27, 2022 14:20
MichaelDeBoey pushed a commit to MichaelDeBoey/web-std-io that referenced this pull request Aug 26, 2023
Co-authored-by: Clifford Fajardo <cfajardo@linkedin.com>
Gozala added a commit to MichaelDeBoey/web-std-io that referenced this pull request Aug 28, 2023
* fix: add typings for HTTPS agent for fetch and Request (remix-run#18)

Co-authored-by: Clifford Fajardo <cfajardo@linkedin.com>

* chore: fix changeset

---------

Co-authored-by: Clifford Fajardo <cliffordfajardo@gmail.com>
Co-authored-by: Clifford Fajardo <cfajardo@linkedin.com>
Co-authored-by: Irakli Gozalishvili <contact@gozala.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants