search

ExodusMovement / fetch

MIT License
0 stars 0 forks source link

feat: replace fetchival with a rewritten version #3

Closed ChALkeR closed 1 year ago

ChALkeR commented 1 year ago

The aim is to add more safeguards and stop using unsupported upstream fetchival at all.

Functional differences between fetchival and this impl are documented in the code:

  1. Also encodeURIComponent the parameter keys, otherwise we can get a problem
  2. Don't pollute the options object that was passed in
  3. Don't silently ignore the body option (fetchival was polluting but ignoring it, we don't pollute and throw instead of ignoring)

Also we will verify that the subpath doesn't contain e.g. .., but that will be done in a separate PR

ChALkeR commented 1 year ago

@feri42 an alternative approach would be to call this /fetchival-compat and opt-in per-asset

feri42 commented 1 year ago

@feri42 an alternative approach would be to call this /fetchival-compat and opt-in per-asset

Not sure what you mean fetchival-compat. But we need to opt-in per asset in any case. Just to make sure everything works.

This PR looks good to me though.

ChALkeR commented 1 year ago

@feri42 how about we use this as a pre-release version for now, update per-asset (and get duplicate versions), once we ensure no issues for those assets, we release it as final and everything else gets updated automatically (without an opt-in)?

Otherwise we might spend forever updating all the places we use fetchival in.

ChALkeR commented 1 year ago

I'll land this as is given the utACK then and release a new beta version.

ChALkeR commented 1 year ago

Done. This is now released as 1.3.0-beta.2

ChALkeR commented 1 year ago

As for wretch -- replacing the API usage should be easier than creating a replacement for the dep.

feri42 commented 1 year ago

@ChALkeR , I suggest we choose one asset, do the replacement, check the asset in all of the clients and if that works, we can do a release. If it works for one asset in all clients, we likely won't have issues with other assets. And if we do, we try to figure out something. Maybe we try with ethereum.