Skip to content

enh: allow refresh times greater than max JS number, of ~24.8 days #891

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

Closed
wants to merge 11 commits into from

Conversation

cip8
Copy link
Contributor

@cip8 cip8 commented Aug 29, 2024

🔗 Linked issue

889 (link)

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

  • Postponing refresh when interval is greater than 24.8 days, to comply with JS max number limit.
  • Increased default refresh interval - 5 mins is a reasonable minimum for most apps & avoids too many calls to backend.

📝 Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

Sorry, something went wrong.

cip8 added 3 commits August 29, 2024 14:45

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian
… with JS max number limit.

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian
…or most apps & avoids too many calls to backend.

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian
… the method is called, but we default to zero just in case.
Copy link

pkg-pr-new bot commented Aug 29, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/@sidebase/nuxt-auth@891

commit: f49294d

@cip8
Copy link
Contributor Author

cip8 commented Aug 29, 2024

Just realized I need to apply the solution for refetchIntervalTimer too - both for consistency and to cover cases where users might prefer to also set enablePeriodically to a very long interval.

Will update this PR soon.

cip8 added 4 commits August 29, 2024 16:26

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian
…for starting refresh timers.

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian
@cip8
Copy link
Contributor Author

cip8 commented Aug 29, 2024

Done - tested with my backend, everything works fine.

Let me know if it needs anything else!

@zoey-kaiser
Copy link
Member

Hi @cip8 👋

Thank you for the PR! I would propose @phoenix-ru does a full review when he gets back from his break, as he wrote / maintains the refresh logic.

Until then feel free to install and use the temporary package created with your last commit as a fix until the PR is merged and released:

pnpm add https://pkg.pr.new/@sidebase/nuxt-auth@b41f424

@zoey-kaiser zoey-kaiser requested a review from phoenix-ru August 29, 2024 15:29
@zoey-kaiser zoey-kaiser added enhancement An improvement that needs to be added provider-refresh An issue with the refresh provider labels Aug 29, 2024
cip8 added 2 commits August 29, 2024 17:35

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian

Verified

This commit was signed with the committer’s verified signature.
cip8 Ciprian
zoey-kaiser and others added 2 commits September 5, 2024 12:53

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@phoenix-ru
Copy link
Collaborator

Closed in favor of #1032

@phoenix-ru phoenix-ru closed this Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement that needs to be added provider-refresh An issue with the refresh provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants