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.

Sign up for GitHub

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

Jump to bottom

refactor(web): drop axios #7490

Merged
merged 6 commits into from Feb 29, 2024
Merged

refactor(web): drop axios #7490

merged 6 commits into from Feb 29, 2024

Conversation

jrasm91
Copy link
Contributor

@jrasm91 jrasm91 commented Feb 28, 2024 •
edited

Closes #6647

  • Drops axios in favor of fetch
  • Migrates the remaining usages to @immich/sdk
  • The 4 remaining endpoints (that rely of signals or progress events) have been temporarily migrated to use and xhr implementation.
  • Tested by uploading and downloading files from albums, shared, links, and the main timeline
Before After
image image

@cloudflare-pages Cloudflare Pages
Copy link

cloudflare-pages bot commented Feb 28, 2024 •
edited

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: b55e7e0
Status:⚡️  Build in progress...

View logs

@jrasm91 jrasm91 force-pushed the refactor/drop-axios branch 2 times, most recently from 30ef20f to f844d32 Compare February 28, 2024 03:22
@michelheusschen
Copy link
Contributor

Nice work! You might even be able to use @immich/sdk for the AlbumCard, PanoramaViewer and PhotoViewer components because they don't use download progress.

@jrasm91
Copy link
Contributor Author

jrasm91 commented Feb 28, 2024

Nice work! You might even be able to use @immich/sdk for the AlbumCard, PanoramaViewer and PhotoViewer components because they don't use download progress.

Oh I think you might be right, since it should support blob response type.

@benmccann benmccann linked an issue Feb 28, 2024 that may be closed by this pull request
PERF: switch from axios to fetch #6647
Closed
3 tasks
benmccann
benmccann reviewed Feb 28, 2024
View reviewed changes
Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

yesss!! this is great!

web/src/lib/components/album-page/__tests__/album-card.spec.ts Outdated Show resolved Hide resolved
martabal
martabal approved these changes Feb 28, 2024
View reviewed changes
Copy link
Member

@martabal martabal left a comment

Choose a reason for hiding this comment

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

Wow, that's great !

@benmccann
Copy link
Contributor

Just FYI, it looks like it needs a rebase

@jrasm91 jrasm91 marked this pull request as ready for review February 29, 2024 16:19
@jrasm91 jrasm91 merged commit 09a7291 into main Feb 29, 2024
27 of 28 checks passed
@jrasm91 jrasm91 deleted the refactor/drop-axios branch February 29, 2024 16:22
This was referenced Mar 20, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieldietzler danieldietzler danieldietzler approved these changes

@martabal martabal martabal approved these changes

@benmccann benmccann Awaiting requested review from benmccann

Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PERF: switch from axios to fetch
5 participants
@jrasm91 @michelheusschen @benmccann @danieldietzler @martabal