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

feat: add @purplet/rest to replace @discordjs/rest #33

Merged
merged 45 commits into from
Jul 14, 2022
Merged

feat: add @purplet/rest to replace @discordjs/rest #33

merged 45 commits into from
Jul 14, 2022

Conversation

paperdave
Copy link
Member

@paperdave paperdave commented Jul 10, 2022

Closes #7 by implementing a rest client. Instead of explicitly relying on node.js or bun directly, we are using web standards and then @purplet/polyfill to add missing standards.

as it stands right now you need the polyfill for both runtimes: node <18 doesn't support fetch and formdata, and bun and node 18 have fetch but not formdata. this will probably get better in the future but idk how long that will be.

I also want to note this has modifications to @purplet/polyfill to get formdata working

@paperdave paperdave added enhancement New feature or request pkg:rest labels Jul 10, 2022
@Hycord Hycord marked this pull request as ready for review July 10, 2022 15:20
@Hycord
Copy link
Contributor

Hycord commented Jul 10, 2022

I'm bad at github, didn't mean to do that lmao

@paperdave
Copy link
Member Author

lmao nice. oh we'll shouldn't make anything different happen

@paperdave
Copy link
Member Author

last two commits implement files with form data. it's significant because the second commit changes the polyfill to mirror the web apis, so the implementation in rest is super simple.

@paperdave paperdave changed the title Create @purplet/rest feat: add @purplet/rest to replace @discordjs/rest Jul 11, 2022
@paperdave
Copy link
Member Author

@purplet/rest is published on the next dist-tag. we could get some testing with it. last step here is replacing rest client in purplet itself.

@paperdave
Copy link
Member Author

ready for code review.

Copy link
Contributor

@Hycord Hycord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be tested before full 1.0.0 release
Otherwise, LGTM

@paperdave paperdave merged commit 0ac1bdd into main Jul 14, 2022
@paperdave paperdave deleted the rest branch July 14, 2022 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg:rest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Our own rest client
2 participants