Skip to content
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

Feature: Add fetch function to utils #187

Merged
merged 3 commits into from
Dec 22, 2023
Merged

Conversation

SoraTenshi
Copy link
Contributor

As already discussed on the Discord, here is my submission for a asynchronous fetch function, that will provide the ability (for now) to add POST as well as GET reqeuests.

My design considerations are mostly, that types should do essentially most of the Documentation and i think it does a pretty good job.

An example usage of this function would be:

import * as Utils from 'resource:///com/github/Aylur/ags/utils.js';

const options = {
  method: 'GET',
  headers: null,
};

Utils.fetch('http://wttr.in/?format=3', options)
  .then(res => {
    res = "[redacted]" + res.substring(res.indexOf(':'));
    console.log(res);
  })
  .catch(err => console.error(err));

This example will just print something like:
Gjs-Console-Message: 16:32:47.627: [redacted]: 🌨 +1°C to the console.

@Aylur
Copy link
Owner

Aylur commented Nov 26, 2023

Shouldn't the options parameter default to { method: 'GET' }?

@SoraTenshi
Copy link
Contributor Author

Shouldn't the options parameter default to { method: 'GET' }?

Oops, yep.
Should have tested the default fallback beforehand.
Fixed with the last push.

@kotontrion
Copy link
Contributor

Currently you decode the received bytes using TextDecoder, but this assumes the received data is actually text. So I can't fetch something like an image using this.
So maybe the byte decoding should look at the content type of the response before decoding? Or is this out of scope for now?

@SoraTenshi
Copy link
Contributor Author

Currently you decode the received bytes using TextDecoder, but this assumes the received data is actually text. So I can't fetch something like an image using this. So maybe the byte decoding should look at the content type of the response before decoding? Or is this out of scope for now?

I would actually go a slightly different with this approach, as in:
provide a 'lower level' method, that makes the user decode the bytes themselves, this is a simple fetch method, i would rather add something like a
fetchBytes bethod or smth alike.
Shouldn't also be difficult to implement, but i can't really test it i think, unless you can help with e.g. setting up something like fetching album cover via mpris, with that i could test and implement such a function. :)

@Aylur
Copy link
Owner

Aylur commented Nov 28, 2023

I haven't used the fetch web api yet, other than to get jsons, so I am not sure how to go about this.
Maybe we should avoid naming it fetch, unless we can make it 1:1 to the web api. I haven't got to testing your PR yet, nor have I looked into how Soup works yet, but how about having different functions, something like fetchJson, fetchBlob, fetchPost?

@SoraTenshi
Copy link
Contributor Author

I haven't used the fetch web api yet, other than to get jsons, so I am not sure how to go about this. Maybe we should avoid naming it fetch, unless we can make it 1:1 to the web api. I haven't got to testing your PR yet, nor have I looked into how Soup works yet, but how about having different functions, something like fetchJson, fetchBlob, fetchPost?

I have talked with @kotontrion about this issue, and i think the best approach to handle multiple different ways to parse the fetched result would be, to have the function take in another (optional) parameter in form of a Decoder function. We might even be able to provide some for basic usecases, e.g. StringDecoder, JSONDecoder, BlobDecoder, ...

i would then appropriately set the 'default' behaviour to string decoding and (if you'd agree with me on that one), and leave the multiple options there (which i have engraved into types)

@SoraTenshi SoraTenshi force-pushed the fetch-util branch 2 times, most recently from 97c51ab to 0ae23a6 Compare November 30, 2023 17:56
@SoraTenshi SoraTenshi force-pushed the fetch-util branch 2 times, most recently from b5d1b3c to c45a175 Compare December 12, 2023 19:27
@SoraTenshi SoraTenshi marked this pull request as draft December 12, 2023 19:54
- Also fix lint
@Aylur
Copy link
Owner

Aylur commented Dec 18, 2023

I saw this linked on Discord https://gist.github.com/derekstavis/82c509789ee7ed0954a88592548dd815
This seems like a fully compatible version

@Aylur Aylur marked this pull request as ready for review December 22, 2023 20:43
@Aylur Aylur merged commit 2022081 into Aylur:main Dec 22, 2023
@SoraTenshi SoraTenshi deleted the fetch-util branch December 22, 2023 21:44
gorsbart pushed a commit to gorsbart/ags that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants