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

feat: remove 3 http related polyfills #2375

Merged
merged 12 commits into from
Jul 26, 2023
Merged

Conversation

ckniffen
Copy link
Collaborator

@ckniffen ckniffen commented Jul 12, 2023

High Level Overview of Change

Switch to using fetch for browser and node-fetch for node for the faucet calls. This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as url, stream-http, and https-browserify.

Context of Change

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Refactor (non-breaking change that only restructures code)

Test Plan

Some new tests might need to be added to validate error checking.

Eliminate the need to specify polyfills for `url`, `http`, and `https`.

Future commits will add a dependency of node-fetch for running in node
which will be handled transparently and in a backward compatible way.

Another large benefit is this substantially simplifies `fundWallet.ts`
# Conflicts:
#	package-lock.json
#	packages/xrpl/src/Wallet/fundWallet.ts
#	packages/xrpl/src/index.ts
@ckniffen ckniffen changed the base branch from main to 3.0 July 17, 2023 17:59
@socket-security
Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
cross-fetch 4.0.0 network +1 250 kB lquixada

@ckniffen ckniffen marked this pull request as ready for review July 17, 2023 20:06
@ckniffen ckniffen added the 3.0 label Jul 17, 2023
@JST5000
Copy link
Contributor

JST5000 commented Jul 17, 2023

FYI I added branch protections for 3.0 (same ones that apply to main)


// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Its a FaucetWallet
const body = (await response.json()) as FaucetWallet
// "application/json; charset=utf-8"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a useful comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dont think so but it was left over.

Copy link
Contributor

@JST5000 JST5000 left a comment

Choose a reason for hiding this comment

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

LGTM

@ckniffen ckniffen changed the title feat: remove https-browserify feat: remove 3 http related polyfills Jul 20, 2023
@ckniffen ckniffen merged commit e41967f into 3.0 Jul 26, 2023
12 checks passed
@ckniffen ckniffen deleted the feature/remove-https-browserify branch July 26, 2023 18:18
ckniffen added a commit that referenced this pull request Aug 12, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Aug 15, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Aug 28, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Aug 28, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Sep 1, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Oct 5, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Oct 18, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Oct 25, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Nov 1, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Nov 30, 2023
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
ckniffen added a commit that referenced this pull request Feb 1, 2024
Switch to using fetch for browser and `node-fetch` for node for the faucet calls.  This reduces the webpack bundle by 3.2% or 16.5kb gzipped.

The fundWallet code has been refactored to be much more straight forward due to not having to do such low level operations.

This improves the frontend setup process by no longer requiring several polyfills such as `url`, `stream-http`, and `https-browserify`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants