feat: remove ky from http-client and utils #2810

hugomrdias · 2020-03-03T14:33:30Z

This PR removes ky and simplifies a couples things searchParams handling, baseUrl normalization and native fetch options usage.

Native fetch options can be passed into the contructor or methods and are merged properly, methods now use the third argument to receive these options.

left some comments in the code to explain some of the ideas, please review so i can continue refactoring if approved.

No support for retries or hooks will implement as needed.

closes #2801

related
https://github.com/ipfs/interface-js-ipfs-core/issues/58

packages/ipfs-http-client/src/index.js

packages/ipfs-http-client/src/add/index.js

achingbrain · 2020-03-04T12:11:48Z

Looks good so far 👍

pcowgill · 2020-03-05T02:04:10Z

packages/ipfs-http-client/src/add/index.js

-    if (options.chunker) searchParams.set('chunker', options.chunker)
-    if (options.cidVersion) searchParams.set('cid-version', options.cidVersion)
-    if (options.cidBase) searchParams.set('cid-base', options.cidBase)
-    if (options.enableShardingExperiment != null) searchParams.set('enable-sharding-experiment', options.enableShardingExperiment)
-    if (options.hashAlg) searchParams.set('hash', options.hashAlg)
-    if (options.onlyHash != null) searchParams.set('only-hash', options.onlyHash)
-    if (options.pin != null) searchParams.set('pin', options.pin)
-    if (options.progress) searchParams.set('progress', true)
-    if (options.quiet != null) searchParams.set('quiet', options.quiet)
-    if (options.quieter != null) searchParams.set('quieter', options.quieter)
-    if (options.rawLeaves != null) searchParams.set('raw-leaves', options.rawLeaves)
-    if (options.shardSplitThreshold) searchParams.set('shard-split-threshold', options.shardSplitThreshold)
-    if (options.silent) searchParams.set('silent', options.silent)
-    if (options.trickle != null) searchParams.set('trickle', options.trickle)
-    if (options.wrapWithDirectory != null) searchParams.set('wrap-with-directory', options.wrapWithDirectory)
-    if (options.preload != null) searchParams.set('preload', options.preload)
-    if (options.fileImportConcurrency != null) searchParams.set('file-import-concurrency', options.fileImportConcurrency)
-    if (options.blockWriteConcurrency != null) searchParams.set('block-write-concurrency', options.blockWriteConcurrency)


It's nice that this is much DRYer now

We can resolve this comment thread

pcowgill · 2020-03-09T17:26:11Z

@hugomrdias What work remains before this PR will leave the draft stage? Thanks!

pcowgill · 2020-03-09T20:09:28Z

packages/ipfs-http-client/src/index.js

+    headers: config.headers,
+    base: normalizeURL(config).toString(),
+    handleError: errorHandler,
+    // apply two mutations camelCase to kebad-case and remove undefined/null key/value pairs


Suggested change

// apply two mutations camelCase to kebad-case and remove undefined/null key/value pairs

// apply two mutations camelCase to kebab-case and remove undefined/null key/value pairs

pcowgill · 2020-03-09T20:11:21Z

packages/ipfs-http-client/src/index.js

+  throw error
+}
+
+var KEBAB_REGEX = /[A-Z\u00C0-\u00D6\u00D8-\u00DE]/g


Might be helpful to insert a comment above this line explaining in plain text what this regex is intended to catch beyond A-Z

@hugomrdias Curious for your thoughts on this when you get a chance. Thanks!

pcowgill · 2020-03-09T20:12:20Z

packages/ipfs-http-client/src/index.js

+      url.port = config.port || location.port
+    } else {
+      url.host = config.host || 'localhost'
+      url.port = config.port || '5001'


Defaults to go-ipfs rather than js-ipfs (5002)?

pcowgill · 2020-03-09T20:24:37Z

Once dag.put is refactored to use the new api.post function I can check whether #2825 is still an issue in this feature branch.

achingbrain

This is a monster. Let's get it in and address any problems with follow up PRs.

packages/ipfs-http-client/src/add/index.js

packages/ipfs-http-client/src/version.js

packages/interface-ipfs-core/src/dht/put.js

pcowgill · 2020-03-11T19:30:23Z

packages/ipfs-http-client/src/lib/core.js

+      headers: opts.headers,
+      base: normalizeInput(opts.url).toString(),
+      handleError: errorHandler,
+      // apply two mutations camelCase to kebad-case and remove undefined/null key/value pairs


Suggested change

// apply two mutations camelCase to kebad-case and remove undefined/null key/value pairs

// apply two mutations camelCase to kebab-case and remove undefined/null key/value pairs

achingbrain · 2020-03-12T07:25:58Z

Let's get it in and address any problems with follow up PRs.

I didn't mean merge while CI was red 🤣

hugomrdias · 2020-03-12T08:03:19Z

fixing it, for some reason i just so green 🤣

- we now use a simpler http lib - bundle size reduced - some tests are now more stable - search params handling is cleaner closes ipfs#2801

- we now use a simpler http lib - bundle size reduced - some tests are now more stable - search params handling is cleaner closes #2801

hugomrdias added the pkg:http-client Issues related only to ipfs-http-client label Mar 3, 2020

hugomrdias self-assigned this Mar 3, 2020

hugomrdias requested a review from achingbrain March 3, 2020 15:03

achingbrain reviewed Mar 3, 2020

View reviewed changes

packages/ipfs-http-client/src/index.js Outdated Show resolved Hide resolved

achingbrain reviewed Mar 3, 2020

View reviewed changes

packages/ipfs-http-client/src/add/index.js Outdated Show resolved Hide resolved

hugomrdias requested a review from achingbrain March 3, 2020 19:52

pcowgill reviewed Mar 5, 2020

View reviewed changes

hugomrdias added 6 commits March 5, 2020 16:22

fix: prototype for ky removal

85b8802

fix: adjustments from feedback

a1a7230

fix: lint and globalThis

b16c080

fix: add missing dep

10c3734

fix: add missing option and defaults

7cfde8e

fix: add validation

185cd8e

pcowgill reviewed Mar 9, 2020

View reviewed changes

pcowgill approved these changes Mar 9, 2020

View reviewed changes

This was referenced Mar 9, 2020

Update ky-universal to the latest version 🚀 #2734

Closed

Update ky to the latest version 🚀 #2723

Closed

fix: removed ky

6a3db3f

This was referenced Mar 11, 2020

urlSource.options is ignored #2807

Closed

chore: move mfs and multipart files into core #2811

Merged

fix: re-add configure

1c619bb

hugomrdias force-pushed the feat/remove-ky branch from 0b7a5fd to 1c619bb Compare March 11, 2020 16:29

hugomrdias added 2 commits March 11, 2020 16:45

fix: remove ky from utils

44ac9e3

fix: fix textdecoder in node 10 and revert http server changes

ef9435d

achingbrain approved these changes Mar 11, 2020

View reviewed changes

packages/ipfs-http-client/src/add/index.js Show resolved Hide resolved

pcowgill reviewed Mar 11, 2020

View reviewed changes

packages/ipfs-http-client/src/version.js Show resolved Hide resolved

hugomrdias marked this pull request as ready for review March 11, 2020 19:11

hugomrdias commented Mar 11, 2020

View reviewed changes

packages/interface-ipfs-core/src/dht/put.js Show resolved Hide resolved

chore: a TODO

9b27599

pcowgill reviewed Mar 11, 2020

View reviewed changes

chore: bundle size and remove comments

cc69bce

hugomrdias changed the title ~~remove ky from http-client~~ feat: remove ky from http-client and utils Mar 11, 2020

hugomrdias merged commit 9bc9625 into master Mar 11, 2020

hugomrdias deleted the feat/remove-ky branch March 11, 2020 19:47

SgtPooki referenced this pull request in ipfs/js-kubo-rpc-client Aug 18, 2022

feat: remove ky from http-client and utils (#2810)

41ea529

- we now use a simpler http lib - bundle size reduced - some tests are now more stable - search params handling is cleaner closes #2801

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove ky from http-client and utils #2810

feat: remove ky from http-client and utils #2810

hugomrdias commented Mar 3, 2020 •

edited

Loading

achingbrain commented Mar 4, 2020

pcowgill Mar 5, 2020

pcowgill Mar 9, 2020

pcowgill commented Mar 9, 2020

pcowgill Mar 9, 2020

pcowgill Mar 9, 2020

pcowgill Mar 18, 2020

pcowgill Mar 9, 2020

pcowgill commented Mar 9, 2020

achingbrain left a comment

pcowgill Mar 11, 2020

achingbrain commented Mar 12, 2020

hugomrdias commented Mar 12, 2020

	// apply two mutations camelCase to kebad-case and remove undefined/null key/value pairs
	// apply two mutations camelCase to kebab-case and remove undefined/null key/value pairs

feat: remove ky from http-client and utils #2810

feat: remove ky from http-client and utils #2810

Conversation

hugomrdias commented Mar 3, 2020 • edited Loading

achingbrain commented Mar 4, 2020

pcowgill Mar 5, 2020

Choose a reason for hiding this comment

pcowgill Mar 9, 2020

Choose a reason for hiding this comment

pcowgill commented Mar 9, 2020

pcowgill Mar 9, 2020

Choose a reason for hiding this comment

pcowgill Mar 9, 2020

Choose a reason for hiding this comment

pcowgill Mar 18, 2020

Choose a reason for hiding this comment

pcowgill Mar 9, 2020

Choose a reason for hiding this comment

pcowgill commented Mar 9, 2020

achingbrain left a comment

Choose a reason for hiding this comment

pcowgill Mar 11, 2020

Choose a reason for hiding this comment

achingbrain commented Mar 12, 2020

hugomrdias commented Mar 12, 2020

hugomrdias commented Mar 3, 2020 •

edited

Loading