Skip to content

feat: do not refresh session if tab is in bg#1280

Open
asafshen wants to merge 3 commits intomainfrom
do-not-refresh-on-bg
Open

feat: do not refresh session if tab is in bg#1280
asafshen wants to merge 3 commits intomainfrom
do-not-refresh-on-bg

Conversation

@asafshen
Copy link
Member

@asafshen asafshen commented Dec 3, 2025

Related Issues

Fixes https://github.com/descope/etc/issues/13001

Description

  • by default, do not do refresh if document is not visible (this is required for session inactivity) but instead mark refresh as needed so when the document visibility changes - do the refresh
  • this may have some subtle bc, so for now I added an option to do pas autoRefresh: { ignoreVisibility: true } to preserve old behavior

still required tests

Copilot AI review requested due to automatic review settings December 3, 2025 08:14
@vercel
Copy link

vercel bot commented Dec 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
access-key-management-widget Ready Ready Preview Comment Dec 4, 2025 1:35pm
audit-management-widget Ready Ready Preview Comment Dec 4, 2025 1:35pm
role-management-widget Ready Ready Preview Comment Dec 4, 2025 1:35pm
user-management-widget Ready Ready Preview Comment Dec 4, 2025 1:35pm
user-profile-widget Ready Ready Preview Comment Dec 4, 2025 1:35pm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces visibility-aware session refresh behavior to prevent unnecessary token refreshes when the browser tab is in the background. This is a key requirement for session inactivity tracking, as refreshing tokens while the tab is hidden would incorrectly extend session lifetimes.

Key Changes:

  • Defers token refresh when tab is hidden, marking it as needed instead
  • Adds ignoreVisibility option to preserve backward compatibility
  • Updates visibility change handler to recalculate timers when tab becomes visible

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/sdks/web-js-sdk/src/enhancers/withAutoRefresh/types.ts Extends autoRefresh config to accept object with ignoreVisibility option
packages/sdks/web-js-sdk/src/enhancers/withAutoRefresh/index.ts Implements deferred refresh logic with refreshNeeded flag and visibility checks
packages/sdks/web-js-sdk/test/autoRefresh.test.ts Updates test to use new config format with ignoreVisibility: true

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// for a case that the token was refreshed from another tab, this mostly relevant
// when the project uses token rotation
sdk.refresh(getRefreshToken() || refreshJwt);
if (!ignoreVisibility && document.visibilityState === 'hidden') {
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The document object may not be defined in non-browser environments despite the IS_BROWSER check at line 40. This timer callback could execute in contexts where document is undefined, causing a runtime error. Add a check for IS_BROWSER or ensure document is defined before accessing visibilityState.

Suggested change
if (!ignoreVisibility && document.visibilityState === 'hidden') {
if (!ignoreVisibility && IS_BROWSER && document.visibilityState === 'hidden') {

Copilot uses AI. Check for mistakes.
@nx-cloud
Copy link

nx-cloud bot commented Dec 3, 2025

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.


View your CI Pipeline Execution ↗ for commit 2f9893f

Command Status Duration Result
nx affected --target test:e2e ❌ Failed 7m 5s View ↗
nx affected --target test ✅ Succeeded 3m 29s View ↗
nx affected --target lint --fix=true ✅ Succeeded 5s View ↗
nx affected --target build ✅ Succeeded 2m 56s View ↗
nx affected --target licenseCheck ✅ Succeeded <1s View ↗

☁️ Nx Cloud last updated this comment at 2025-12-04 13:51:24 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants