feat: experimental fetchival replacement

[ChALkeR]

This is an experimental introduction of an opt-in safe fetchival replacement.

Unlike fetchival:

• timeout defaults to 30s everywhere instead of 300s in browser and 30s in node-fetch
• fetchival.fetch is now undefined and isn't used
• first parameter of fetchival(url) should always be an URL instance
• fetchival(url)(subpath) always expects subpath to not include / symbol
• urls are checked to not include ?/& or #, as parameters are set via options
• url merging auto-detects / separator, e.g. a + b = a/b (like in fetchival), a/ + b = a/b (not a//b, unlike in fetchival)
• url merging can't use .. or similar

Uses safe URL concatenation from #4.

This aims to replace approach in https://github.com/ExodusMovement/assets/pull/927

This can be used/tested, once we confirm this works, /experimental prefix would be replaced with something else, but an alias would be kept for compat.

[ChALkeR]

Diff with current/default implementation:

1c1
< // Based on https://github.com/typicode/fetchival and aims to keep most of the same API
---
> // API somewhat based on https://github.com/typicode/fetchival, but with significant changes
3,9c3,4
< // Unlike fetchival, we also encode the keys
< function query(params) {
<   if (!params) return ''
<   return `?${Object.entries(params)
<     .map(([k, v]) => `${encodeURIComponent(k)}=${encodeURIComponent(v)}`)
<     .join('&')}`
< }
---
> const { url } = require('../url')
> const fetch = require('../fetch')
11c6
< async function _fetch(method, url, opts, data) {
---
> async function _fetch(method, link, opts, data) {
16c11,12
<   const res = await fetchival.fetch(url, {
---
>   const res = await fetch(link, {
>     timeout: 30000, // default timeout to 30s, unlike fetchival or fetch
39c35,41
< function fetchival(url, opts = {}) {
---
> function fetchival(link, opts = {}) {
>   if (!(link instanceof URL)) throw new TypeError('Url should be an instance of URL')
> 
>   const str = `${link}`
>   if (str.includes('?') || str.includes('&')) throw new Error('Invalid url with params!')
>   if (str.includes('#')) throw new Error('Invalid url with hash!')
> 
41,42c43,46
<     // TODO: validate subpath here
<     return fetchival(`${url}/${sub}`, { ...opts, ...o })
---
>     // Unlike fetchival, this performs additional validation
>     if (sub.includes('/')) throw new Error('Only simple subpaths are allowed!')
>     const joined = str.endsWith('/') ? url`${link}${sub}` : url`${link}/${sub}`
>     return fetchival(joined, { ...opts, ...o })
45,49c49,53
<   _.get = (params) => _fetch('GET', url + query(params), opts)
<   _.post = (data) => _fetch('POST', url, opts, data)
<   _.put = (data) => _fetch('PUT', url, opts, data)
<   _.patch = (data) => _fetch('PATCH', url, opts, data)
<   _.delete = () => _fetch('DELETE', url, opts)
---
>   _.get = (params) => _fetch('GET', params ? url`${link}?${params}` : link, opts)
>   _.post = (data) => _fetch('POST', link, opts, data)
>   _.put = (data) => _fetch('PUT', link, opts, data)
>   _.patch = (data) => _fetch('PATCH', link, opts, data)
>   _.delete = () => _fetch('DELETE', link, opts)
53,54d56
< 
< fetchival.fetch = typeof fetch !== 'undefined' ? fetch.bind(globalThis) : null

[feri42]

fetchival.fetch is now undefined and isn't used

We do use fetchival.fetch in the clients: https://github.com/ExodusMovement/exodus-desktop/blob/0b4e42c36fac5893c7f6cc6af66a8b51e5265713/src/app/ui/app-actions/monitors/remote-config/index.js#L71 . However maybe we wouldn't need to if we supported the HEAD request in our fetchival replacement

[ChALkeR]

@feri42 it should be just fetch, which is a separate import.

Also, yes, we can add head support!

[feri42]

@feri42 it should be just fetch, which is a separate import.

Also, yes, we can add head support!

Head support would be nice. Seems strange to me if we used two clients for the same (remote-config) operation. But it would work, of course.

[feri42]

add HEAD support to experimental fetchival replacement

:heart: