Skip to content

Commit

Permalink
ref(rr6): Move GuideStore url update to a react effect (#76690)
Browse files Browse the repository at this point in the history
This removes the usage of very-early `browserHistory.listen`. We can
shim the browserHistory.listen, but only once we've created a react
router 6. In new react router 6 land we don't set the browserHistory
until once the app starts. In react router 3 land it would be
immediately available.

To fix this we just call the `onURLChange` from the main App component.
  • Loading branch information
evanpurkhiser authored Aug 28, 2024
1 parent 461f6c4 commit c099a85
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
7 changes: 0 additions & 7 deletions static/app/stores/guideStore.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import HookStore from 'sentry/stores/hookStore';
import ModalStore from 'sentry/stores/modalStore';
import type {Organization} from 'sentry/types/organization';
import {trackAnalytics} from 'sentry/utils/analytics';
import {browserHistory} from 'sentry/utils/browserHistory';

import type {StrictStoreDefinition} from './types';

Expand Down Expand Up @@ -86,7 +85,6 @@ function isForceEnabled() {
}

interface GuideStoreDefinition extends StrictStoreDefinition<GuideStoreState> {
browserHistoryListener: null | (() => void);
closeGuide(dismissed?: boolean): void;

fetchSucceeded(data: GuidesServerData): void;
Expand All @@ -106,7 +104,6 @@ interface GuideStoreDefinition extends StrictStoreDefinition<GuideStoreState> {

const storeConfig: GuideStoreDefinition = {
state: {...defaultState},
browserHistoryListener: null,
modalStoreListener: null,

init() {
Expand All @@ -116,7 +113,6 @@ const storeConfig: GuideStoreDefinition = {
this.state = {...defaultState, forceShow: isForceEnabled()};

window.addEventListener('load', this.onURLChange, false);
this.browserHistoryListener = browserHistory.listen(() => this.onURLChange());

// Guides will show above modals, but are not interactable because
// of the focus trap, so we force them to be hidden while a modal is open.
Expand All @@ -134,9 +130,6 @@ const storeConfig: GuideStoreDefinition = {
teardown() {
window.removeEventListener('load', this.onURLChange);

if (this.browserHistoryListener) {
this.browserHistoryListener();
}
if (this.modalStoreListener) {
this.modalStoreListener();
}
Expand Down
6 changes: 6 additions & 0 deletions static/app/views/app/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import Indicators from 'sentry/components/indicators';
import {DEPLOY_PREVIEW_CONFIG, EXPERIMENTAL_SPA} from 'sentry/constants';
import AlertStore from 'sentry/stores/alertStore';
import ConfigStore from 'sentry/stores/configStore';
import GuideStore from 'sentry/stores/guideStore';
import HookStore from 'sentry/stores/hookStore';
import OrganizationsStore from 'sentry/stores/organizationsStore';
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
Expand All @@ -27,6 +28,7 @@ import useApi from 'sentry/utils/useApi';
import {useColorscheme} from 'sentry/utils/useColorscheme';
import {GlobalFeedbackForm} from 'sentry/utils/useFeedbackForm';
import {useHotkeys} from 'sentry/utils/useHotkeys';
import {useLocation} from 'sentry/utils/useLocation';
import {useUser} from 'sentry/utils/useUser';
import type {InstallWizardProps} from 'sentry/views/admin/installWizard';
import {AsyncSDKIntegrationContextProvider} from 'sentry/views/app/asyncSDKIntegrationProvider';
Expand Down Expand Up @@ -132,6 +134,10 @@ function App({children, params}: Props) {
}
}, [orgId, sentryUrl, isOrgSlugValid]);

// Update guide store on location change
const location = useLocation();
useEffect(() => GuideStore.onURLChange(), [location]);

useEffect(() => {
loadOrganizations();
checkInternalHealth();
Expand Down

0 comments on commit c099a85

Please sign in to comment.