FujoWebDev / AO3.js

NodeJS API for scraping AO3 data
MIT License
15 stars 8 forks source link

Add Axios overrides #57

Closed hkamran80 closed 10 months ago

hkamran80 commented 10 months ago

This PR adds the optional axiosOptions to the work and series getters.

essential-randomness commented 10 months ago

Thank you so much for this, I'll be taking a look tomorrow or Sunday!

Out of curiosity, are you building something with this?

hkamran80 commented 10 months ago

Thank you!

I am, yes! I'm building a custom notification service because I don't like email notifications.

essential-randomness commented 10 months ago

This potentially works for me, though I'm wondering whether there's a way to do this that doesn't need to add the instance to every method. I can think of two ways off the top of my head:

Setter/getter methods

We set a global axios instance that the library will use rather than passing it every time. This is probably feasible with a module exporting both a setAxiosInstance and a getAxiosInstance like:

import axios from "axios";
import { setAxiosInstance } from "@bobaboard/ao3.js";

const instance = axios.create({
  // options
});

setAxiosInstance(instance);

and then in our fetchers:

import { getAxiosInstance } from "axios-internal";

export const getWhatever = () => {
   getAxiosInstance().get(/* params */);
   // ...
}

Use the axios global default configs

(see: https://axios-http.com/docs/config_defaults)

import axios from "axios";
import { getWork } from "@bobaboard/ao3.js";

axios.defaults.someProperty = /* whatever */;
// other properties override

The disadvantage of this one is that if the library user needs different axios defaults, then they might run into issues.


I'm fine with either, or with merging your work as is (especially if you're ok with the risk of us potentially changing it at a later time, should we figure a better way).

Let me know, and do share your work once you're done if you want! It sounds amazing, and I'd love to show people in my communities :)

hkamran80 commented 10 months ago

I couldn't figure out how to get a global getter/setter working. I also tried the Axios defaults configuration, but for what I needed to do (handle 429 Retry-After errors), it wouldn't work.

I'm fine with merging it now and having it changed later. I will be able happy to share my work once I can get it up and running!

essential-randomness commented 10 months ago

Sounds great! Merging now and releasing :)

essential-randomness commented 10 months ago

Release done! Bumped two versions cause I forgot to rebuild :)

hkamran80 commented 10 months ago

Awesome! Thank you!

essential-randomness commented 8 months ago

FYI, I just opened a new PR that switches axios out for native fetch: #61 It also adds the ability to override the fetcher globally (rather than passing it in every function), and documentation on how to take care of rate limiting and caching.

I'm not going to bump to a major version when I release next since we're still in the 0.x.x range, where even minor releases might be breaking. Wanted to give you a heads up, and just letting you know to pin the library version if things get weird and you don't want to deal with the switch.

If you need help/want to discuss further changes, happy to chat as usual!

hkamran80 commented 8 months ago

Perfect! Thanks for the heads up!