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.

Sign up for GitHub

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

Jump to bottom

1124 request restore ability to inject custom fetch instance #1535

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

1124 request restore ability to inject custom fetch instance #1535

wants to merge 4 commits into from
+55 −12

Conversation

lucanicoladebiasi
Copy link
Collaborator

@lucanicoladebiasi lucanicoladebiasi commented Nov 25, 2024 •
edited
Loading

Description

The class packages/network/src/http/SimpleHttpClient.ts introduces the possibility to sets headers in the SimpleHttpClient constructor.

The headers of built objects are included in every request made through the http method, if not overwritten by the params argument of the class packages/network/src/http/HttpParams.ts.

This solutions allows to set headers to manage HTTP cookies and control-caching.

See the test 'ok <- GET and validate at packages/network/tests/http/SimpleHttpClient.solo.test.ts.

Fixes #1124

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change introduced a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • yarn test:solo
  • yarn test:unit

Test Configuration:

  • Node.js Version: v23.1.0
  • Yarn Version: 1.22.22

Checklist:

  • My code follows the coding standards of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • New and existing integration tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have not added any vulnerable dependencies to my code

@lucanicoladebiasi lucanicoladebiasi self-assigned this Nov 25, 2024
@lucanicoladebiasi lucanicoladebiasi linked an issue Nov 25, 2024 that may be closed by this pull request
💡 [REQUEST] - Restore ability to inject custom "fetch" instance #1124
Open
@lucanicoladebiasi lucanicoladebiasi added this to the 1.0.0-RC.3 milestone Nov 25, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 25, 2024 •
edited
Loading

Test Coverage

Summary

Lines Statements Branches Functions
Coverage: 99%
 99.02% (4351/4394) 97.55% (1394/1429) 99.01% (903/912)
Title Tests Skipped Failures Errors Time
core 827 0 💤 0 ❌ 0 🔥 2m 20s ⏱️
network 720 0 💤 0 ❌ 0 🔥 4m 51s ⏱️
errors 42 0 💤 0 ❌ 0 🔥 16.75s ⏱️
logging 3 0 💤 0 ❌ 0 🔥 17.876s ⏱️
hardhat-plugin 19 0 💤 0 ❌ 0 🔥 59.39s ⏱️
aws-kms-adapter 23 0 💤 0 ❌ 0 🔥 1m 10s ⏱️
ethers-adapter 5 0 💤 0 ❌ 0 🔥 1m 14s ⏱️
rpc-proxy 37 0 💤 0 ❌ 0 🔥 1m 2s ⏱️

@lucanicoladebiasi lucanicoladebiasi marked this pull request as ready for review November 25, 2024 17:10
@lucanicoladebiasi lucanicoladebiasi requested a review from a team as a code owner November 25, 2024 17:10
@lucanicoladebiasi lucanicoladebiasi mentioned this pull request Nov 25, 2024
Open
freemanzMrojo
freemanzMrojo approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@freemanzMrojo freemanzMrojo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, left 1 minor comment.

packages/network/tests/thor-client/ThorClient.unit.test.ts
/**
* ThorClient module tests.
*
* @group integration/clients/thor-client
Copy link
Member

Choose a reason for hiding this comment

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

Minor: I think this one should be unit instead of integration.

packages/network/tests/http/SimpleHttpClient.solo.test.ts Show resolved Hide resolved
@victhorbi victhorbi modified the milestones: 1.0.0-RC.3, v1.0.0-RC4 Nov 26, 2024
@claytonneal
Copy link
Member

hi @lucanicoladebiasi i dont see any tests for the new headers, how can u test that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@freemanzMrojo freemanzMrojo freemanzMrojo approved these changes

@claytonneal claytonneal Awaiting requested review from claytonneal

Assignees

@lucanicoladebiasi lucanicoladebiasi

Labels
Priority: High Size: Small Type: Enhancement Type: Feature
Projects
None yet
Milestone
v1.0.0-RC4
Development

Successfully merging this pull request may close these issues.

💡 [REQUEST] - Restore ability to inject custom "fetch" instance
4 participants
@lucanicoladebiasi @claytonneal @freemanzMrojo @victhorbi