Skip to content

feat(remix): add connection check to example page #967

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

Merged
merged 4 commits into from
Apr 22, 2025

Conversation

shellmayr
Copy link
Member

Before:
Screenshot 2025-04-17 at 13 35 07

After:
Screenshot 2025-04-17 at 13 35 14

Copy link

github-actions bot commented Apr 17, 2025

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 6975c2a

Copy link

codecov bot commented Apr 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 24.28%. Comparing base (04f3500) to head (6975c2a).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #967   +/-   ##
=======================================
  Coverage   24.28%   24.28%           
=======================================
  Files         121      121           
  Lines       12883    12883           
  Branches      715      715           
=======================================
  Hits         3128     3128           
  Misses       9734     9734           
  Partials       21       21           
Flag Coverage Δ
unit-tests 24.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shellmayr shellmayr marked this pull request as ready for review April 17, 2025 11:39
@shellmayr shellmayr requested a review from a team April 17, 2025 11:39
setIsConnected(result !== 'sentry-unreachable');
}
checkConnectivity();
}, []);
Copy link
Member

Choose a reason for hiding this comment

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

l: let's avoid potential eslint warnings about empty useEffect arrays

Suggested change
}, []);
}, [setIsConnected]);

Copy link
Member

Choose a reason for hiding this comment

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

Btw some context here (we discussed this offline already, but just if someone is interested): The set... functions have a stable reference so they won't cause useEffect to fire again. However, it's advised to always include all potential variables to the dependency array.

React docs about set... and stable identities: https://react.dev/reference/react/useState#setstate-caveats

@shellmayr shellmayr merged commit aefa4a2 into master Apr 22, 2025
40 checks passed
@shellmayr shellmayr deleted the shellmayr/feat/add-bad-connection-detection-remix branch April 22, 2025 06:11
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.

3 participants