v2.0 Tracking Issue

[Venefilyn]

As mentioned in #262 request is deprecated. If we are replacing it with something else it's a breaking change and needs to be done as a major release, thus tracking towards v2.0.

An incomplete list of things to be done before v2.0

• [ ] Replace deprecated request dependency, axios may be the best contender. fetch may not work as intended for the test suite but would otherwise be the clear winner.
• [ ] Remove deprecated endpoints
• [ ] Look into what to do with Proxies and TypeScript #221
• [ ] Remove underscore function calls maps

[mercmobily]

Just my 2c... I loooovveeeee axios. I am not associated in any way with them -- I just think it's absolutely awesome.

[maximousblk]

any plans on supporting Deno?

[Venefilyn]

any plans on supporting Deno?

I think you should create another issue if something isn't working with Deno. I've got no experience using it so I don't know what doesn't work and for what reasons. So I can't tell whether supporting Deno is easy, difficult, requires breaking changes, and so on

[iMrDJAi]

As mentioned in #262 request is deprecated. If we are replacing it with something else it's a breaking change and needs to be done as a major release, thus tracking towards v2.0.

An incomplete list of things to be done before v2.0

• [ ] Replace deprecated request dependency, axios may be the best contender. fetch may not work as intended for the test suite but would otherwise be the clear winner.
• [ ] Remove deprecated endpoints
• [ ] Look into what to do with Proxies and TypeScript Type error with async/await  #221
• [ ] Remove underscore function calls maps

@SpyTec

1. I think axios would be good for the purpose.

2. Also it's important to add the missing endpoints including the undocumented ones.

3. IMO proxies should be removed, .fetch() methods are what should be used to fetch content. I think the library API should follow this standard:

   const subreddit = requester.getSubreddit("r/redditdev")
   await subreddit.fetch()
   

   Why? The first call won't fetch the subreddit, it will just provide a dummy version of the subreddit object, this will be useful when adding placeholders if you want to render it for example, since you can't actually use promises in this case. The second call will fetch the subreddit, then you can re-render it to display the fetched content instead of the placeholders. The same thing would apply for other use-cases.

4. OK!

[iMrDJAi]

Edit: So after some testing I found out that axios isn't the best choice, it's true that it has a simplified API, but it doesn't actually cover all features if we need to handle everything internally.
The problem is: axios doesn't support "no-cors" mode, in fact it doesn't provide any option to set the request mode, and this would be a problem.

Why? I aim to support uploading media to Reddit, this opens the ability to submit images, videos, galleries and inline media for self-posts. Reddit stores media in Amazon AWS servers, to upload anything you should perform a POST request to the following domain: reddit-uploaded-media.s3-accelerate.amazonaws.com (See https://github.com/not-an-aardvark/snoowrap/issues/280#issuecomment-879916824), which is a thing you cannot do normally in browsers because you will face a CORS error. However it's still possible if you set the request mode to "no-cors", in this case you won't be able to access the response or the status code, but this is more than enough for the purpose.

We should use the Fetch API in browsers, and node-fetch for node, keep in mind this has pros and cons:

Pros:

• Has everything we need.
• Reducing the browser bundle size since we won't include any third party HTTP client.
• Node fetch is the most light-weight HTTP client for node.

Cons:

• More complicated API compared by axios but this wouldn't be an issue.
• We may not support some legacy browsers.

@not-an-aardvark @SpyTec What do u think?

[iMrDJAi]

Edit 2: I will stick to axios!

Legacy browsers support is important, people may use this package to build web applications, some of them will target older browsers, in this case axios is what we should use, it extends XMLHttpRequest which is widely supported, the Fetch API is relatively new so we can't depend on it.

Also, using fetch won't reduce the dependencies size as I thought at the beginning, in order to set a request timeout we should use AbortController which we need to install separately, also fetch doesn't handle request params for us, we have to do it ourselves using url.searchParams which requires a polyfill to work on older browsers.

We're not limited to axios anyway, of course we can use the Fetch API along with it on browser for media uploading, we should do a check to see if it's available on the production environment first, we will omit that on node, and we will drop support for this upcoming feature on older browsers.

[Venefilyn]

@iMrDJAi All good points, we will definitely be removing proxies as they just create more trouble than they're worth.

Adding undocumented endpoints is something I'm not too keen on since they're undocumented. I could see it happening if we make it clear it's undocumented by having method names like UNSAFE_getUndocumentedEndpoint() or similar.

As far as replacing requests goes, we should be respecting CORS while in a browser. Whatever solution we pick will not be affected by CORS while running in Node, it will only have an affect in the browser.

With supporting older browsers, this isn't a concern we should tackle as majority of browsers currently used (95%) support ES6. Market percentage is most likely higher for users of Reddit. Any developers who need to target older browsers can also use polyfills, which means we can target even newer ECMAScript features if need be.

Experimenting with fetch seems like the logical next step to experiment with, and to make that possible both client- and server-side something like cross-fetch could be used. Last time I tried however I did run into issues with the unit tests, but I don't remember what exactly nor can I find any issues where I documented it (whoops).

With AbortController, it is also something that 92% of all browsers currently in use support, people can polyfill these as well no problem if they wish to target older browsers

[iMrDJAi]

@SpyTec

All good points, we will definitely be removing proxies as they just create more trouble than they're worth.

Cool! Another thing should be also removed is bluebird, we should use async/await instead, that's a thing I still working on on my fork.

Any developers who need to target older browsers can also use polyfills

You're right, as long as we will add a notice for developers about possible polyfills they have to use.

something like cross-fetch could be used.

I didn't know cross-fetch before, it seems to be a good solution, actually I liked the way it handles cross environment support. However I been trying with axios on my fork and it worked perfectly! You should take a look at axios.js, I tried my best to provide a maximum backward compatibility with request!

Adding undocumented endpoints is something I'm not too keen on since they're undocumented. I could see it happening if we make it clear it's undocumented by having method names like UNSAFE_getUndocumentedEndpoint() or similar.

Fair enough! We should notify users about methods that communicate with undocumented endpoints, those should be flagged as unstable since they are subject to change. Also probably we should take a look at the PRAW experience to see how the developers of that library handled this situation.

[OmgImAlexis]

Legacy browsers support is important, people may use this package to build web applications, some of them will target older browsers, in this case axios is what we should use, it extends XMLHttpRequest which is widely supported, the Fetch API is relatively new so we can't depend on it.

Why not just ship with fetch and if someone needs XMLHttpRequest support they can add a pollyfill? There are ones that have the same API as fetch but use XMLHttpRequest under the hood.

[iMrDJAi]

@OmgImAlexis Yeah I didn't think about it at the beginning. I realized that we can just let developers install polyfills separately instead of including them on the library.
Anyway, I already made a progress with axios, I even added support for some old properties from the request API to gain a maximum backward compatibility.

[jamesrswift]

3. IMO proxies should be removed, .fetch() methods are what should be used to fetch content. I think the library API should follow this standard:

   const subreddit = requester.getSubreddit("r/redditdev")
   await subreddit.fetch()
   

I think a neat solution might be that proxies are simply of type Partial<SnoowrapObject>, and fetch returns a full SnoowrapObject. This would allow for more type safety as it would become obvious at compile time whether a variable is being accessed before it is assigned.

[amosbastian]

Is this still ongoing? I'd like to contribute if it is!