Replace axios with native fetch #564
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const { data } = await axios.get(config.responseUrl); | ||
| return data; | ||
| }, | ||
| async () => fetch(config.responseUrl).then((r) => r.json()), |
Check warning
Code scanning / CodeQL
File data in outbound network request Medium
mgwalker
force-pushed
the
remove-axios
branch
from
a04cd9e to
b2b9205
Compare
| axios.get.mockResolvedValue({ | ||
| data: ` | ||
| fetch.mockResolvedValue({ | ||
| text: async () => ` |
There was a problem hiding this comment.
ok this looked weird but i looked at the whole file in context and figured it out
There was a problem hiding this comment.
I'll add a comment that it needs to be formatted like yaml so maybe it looks weird to the next person. Would that help?
There was a problem hiding this comment.
i don't think it's necessary, it was easy enough to see when i opened the file!
| .then((response) => { | ||
| if (response && response.status) { | ||
| if (response?.ok && response.status) { |
There was a problem hiding this comment.
ahh i didn't know js had a null-chaining operator!
| "https://www.opm.gov/json/operatingstatus.json", | ||
| ); | ||
| if (status !== 200) { |
There was a problem hiding this comment.
[question] can we get rid of this error throw because data is more consistent now?
Axios doesn't really provide us much benefit over native fetch, so yoink it out and that's one less dependency to worry about!
Will update dev documentation just prior to merging.
Checklist:
The OAuth wiki page has been updated if Charlie needs any new OAuth events or scopesThe Environment Variables wiki page has been updated if new environment variables were introduced or existing ones changedlocal development processes have changedmajor development workflows have changedinternal utilities or APIs have changedIf appropriate, the NIST 800-218 documentation has been updated