-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[MAL] General improvements + API reworks #16982
base: main
Are you sure you want to change the base?
Conversation
Thank you for your first contribution! 🎉 🔔 @luknl you might want to have a look. You can use this guide to learn how to check out the Pull Request locally in order to test it. Due to our current reduced availability, the initial review may take up to 10-15 business days |
This is my first time contributing to an extension, I apologize if this is the right way to publish it. Also, this change is quite significant, I rewrote most of the API to use MALs authenticated API using my own clientId. Will be fixing some of the issues from CI, however I'm unable to run linting since |
Hi @KibbeWater and thanks for your contribution, you did a big job there!
|
afaik there should be no errors caused by this (unless you got any?). All anime's returned thus far have had a PG rating in my testing, but honestly hard to tell since return data is so terribly documented through their APIs.
I have tried looking up mentions of rate-limiting through their API, however seems to be none. From my testing it will temporarily block you if you excessively query the API at unreasonable amounts (like what you'd see during testing or development), but should be fine for normal use. Even then I believe it should only be at the users IP level and not throttle all queries for that application as PKCE OAuth is made to work in environments where you can't trust the client
Yes, OAuth PKCE is made for environments like standalone apps where you can't trust the client |
Ok thanks for all your changes. Also potential further improvements I see possible are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also adding 2 minor change requests
I think so, or some sort of throttling from CloudFlare. I often get hiccups like that after I have been calling the API a lot, maybe the caching I put in place isn't working? Will have to dig a bit deeper
I went ahead and added something on the line of this, similar to the details view it has to override the preference values cause we can't directly set it. Maybe we could have some sort of local storage solution so that we can persist the state even after closing out of the command. As it is now, it's a bit jarring when you switch view, go out and come back into the old view again, but I don't either wanna remove the options from extension settings since I still want it to prompt users for preference on first-time use I'll probably see if I can feature request so that we get some sort of API for actually setting these preference values through code somehow |
Was probably a very large oversight on my end, since my watchlist isn't particularly big, it was able to load just fine even with the excessive API calls. Since we require extra data for each anime in a watchlist to properly display it, we had to query each anime individually. Wasn't a problem for me but found that one of the functions didn't cache properly and made it have to re-query every time you viewed the list The rate-limiting should be mostly resolved, but will put in feature requests on their forums to allow for more bulk querying so that we don't have to resort to spamming their API with a bunch of requests every time we need up-to-date information about a users watchlist |
Ok another batch of changes, nice, thanks again for your contribution! About this error, here is what I have, that might help solving the issue:
When I first pulled, the manage watchlist was working, so maybe it started with the implementation of the caching system. |
Haven't had much time to work on the PR the past few days. How big approximately are your lists? Will probably add some debug views to monitor the number of API calls or something, may prove useful for debugging. Could maybe reduce API calls by using the Jikan API for some calls using batching if they support it, could theoretically get it down from O(n*2) calls to O(n+1) The PUT requests for the item progress would still be required however |
I have 139 in my PTW. But maybe it's checking other types? |
I'll add a bot review too so we can see what it finds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR completely overhauls the MyAnimeList Search extension by replacing Jikan with the official MyAnimeList API and adding OAuth authentication for personalized features.
- Added OAuth authentication in
src/api/oauth.ts
with PKCE flow for secure user authentication - Implemented comprehensive caching system in
src/api/api.ts
to improve performance and reduce API calls - Created new "Manage Watchlist" command with both list and grid views for managing anime collections
- Added context-based view switching between list and grid layouts using
ViewTypeCtx
for better user experience - Improved error handling throughout the codebase with proper try/catch blocks and user-friendly error messages
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
19 file(s) reviewed, 16 comment(s)
Edit PR Review Bot Settings | Greptile
extensions/myanimelist-search/src/components/manage-watchlist/manageWatchList.tsx
Outdated
Show resolved
Hide resolved
extensions/myanimelist-search/src/components/manage-watchlist/manageWatchList.tsx
Outdated
Show resolved
Hide resolved
extensions/myanimelist-search/src/components/manage-watchlist/utils.tsx
Outdated
Show resolved
Hide resolved
extensions/myanimelist-search/src/components/manage-watchlist/manageWatchGrid.tsx
Outdated
Show resolved
Hide resolved
extensions/myanimelist-search/src/components/manage-watchlist/manageWatchGrid.tsx
Outdated
Show resolved
Hide resolved
Yeah, probably checks out why I never had any problems. Thinking about implementing some sort of paging, along with moving our details fetches to jikan with a more clear rate-limiting policy Unfortunately we still need to use the MAL api for the authenticated PUT requests, but total requests should be halved and greatly reduced if we start doing pagination. Also thanks @pernielsentikaer, will have a look at fixing the issues raised by the bot |
Ok after pulling the last changes the "manage watchlist" is working as planned, awesome! About pagination, Raycast support that quite well, so it would be easy to fetch like 50 items per call. And yes thanks @pernielsentikaer for that, it's an amazing tool |
You can read more about the pagination here, is this ready to be merged yet? |
Description
Added a new reworked API, uses the official My Anime List API instead of jikan.
Added OAuth for managing and/or personalizing request and watchlist queries
Screencast
The new command features a fairly visually similar experience to the existing command, don't believe new screenshots are required but can provide if need be
Checklist
npm run build
and tested this distribution build in Raycastassets
folder are used by the extension itselfREADME
are placed outside of themetadata
folder