Make usePageContext return the correct path no matter where its cal…

…led (#21433) ## Summary & Motivation To log page load we need to inform datadog when a new view starts. To do this we currently rely on `usePageContext` to change when the path changes. Currently it doesn't do that because `usePageContext` relies on `useRouteMatch` from react-router-dom which finds the path from the closest parent `Route`. The issue is that we're calling `usePageContext` from a provider that is very high up in the tree, so `useRouteMatch` can't access the context of the `<Route>` currently being rendered. Instead of using `useRouteMatch` lets store the path in global state when `useTrackPageView` is called and have `usePageContext` rely on that global state. Cloud requires this change to dedupe recoil: dagster-io/internal#9471 ## How I Tested These Changes 👀 , play around with the app make sure nothing is broken
dagster-io · Apr 26, 2024 · 8247864 · 8247864 · github-actions · Apr 26, 2024
1 parent 4f5139a
commit 8247864
Show file tree

Hide file tree

Showing 8 changed files with 159 additions and 102 deletions.
diff --git a/js_modules/dagster-ui/packages/app-oss/package.json b/js_modules/dagster-ui/packages/app-oss/package.json
@@ -22,6 +22,7 @@
     "react-router": "^5.2.1",
     "react-router-dom": "^5.3.0",
     "react-router-dom-v5-compat": "^6.3.0",
+    "recoil": "^0.7.7",
     "styled-components": "^5.3.3",
     "uuid": "^9.0.0",
     "validator": "^13.7.0",

diff --git a/js_modules/dagster-ui/packages/app-oss/src/App.tsx b/js_modules/dagster-ui/packages/app-oss/src/App.tsx
@@ -10,6 +10,7 @@ import {logLink, timeStartLink} from '@dagster-io/ui-core/app/apolloLinks';
 import {DeploymentStatusType} from '@dagster-io/ui-core/instance/DeploymentStatusProvider';
 import {LiveDataPollRateContext} from '@dagster-io/ui-core/live-data-provider/LiveDataProvider';
 import {Suspense} from 'react';
+import {RecoilRoot} from 'recoil';
 
 import {InjectedComponents} from './InjectedComponents';
 import {CommunityNux} from './NUX/CommunityNux';
@@ -40,21 +41,23 @@ const appCache = createAppCache();
 // eslint-disable-next-line import/no-default-export
 export default function AppPage() {
   return (
-    <InjectedComponents>
-      <LiveDataPollRateContext.Provider value={liveDataPollRate ?? 2000}>
-        <AppProvider appCache={appCache} config={config}>
-          <AppTopNav allowGlobalReload>
-            <HelpMenu showContactSales={false} />
-            <UserSettingsButton />
-          </AppTopNav>
-          <App>
-            <ContentRoot />
-            <Suspense>
-              <CommunityNux />
-            </Suspense>
-          </App>
-        </AppProvider>
-      </LiveDataPollRateContext.Provider>
-    </InjectedComponents>
+    <RecoilRoot>
+      <InjectedComponents>
+        <LiveDataPollRateContext.Provider value={liveDataPollRate ?? 2000}>
+          <AppProvider appCache={appCache} config={config}>
+            <AppTopNav allowGlobalReload>
+              <HelpMenu showContactSales={false} />
+              <UserSettingsButton />
+            </AppTopNav>
+            <App>
+              <ContentRoot />
+              <Suspense>
+                <CommunityNux />
+              </Suspense>
+            </App>
+          </AppProvider>
+        </LiveDataPollRateContext.Provider>
+      </InjectedComponents>
+    </RecoilRoot>
   );
 }
diff --git a/js_modules/dagster-ui/packages/ui-core/package.json b/js_modules/dagster-ui/packages/ui-core/package.json
@@ -62,6 +62,7 @@
     "react-is": "^18.2.0",
     "react-markdown": "^8.0.6",
     "react-virtualized": "^9.22.3",
+    "recoil": "^0.7.7",
     "rehype-highlight": "^6.0.0",
     "rehype-sanitize": "^5.0.1",
     "remark": "^14.0.2",

diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/analytics.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/analytics.test.tsx
@@ -2,6 +2,7 @@ import {render, screen, waitFor} from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import * as React from 'react';
 import {MemoryRouter, Route, Switch} from 'react-router-dom';
+import {RecoilRoot} from 'recoil';
 
 import {AnalyticsContext, GenericAnalytics, useTrackEvent, useTrackPageView} from '../analytics';
 
@@ -32,15 +33,17 @@ describe('Analytics', () => {
       const mockAnalytics = createMockAnalytics();
 
       render(
-        <MemoryRouter initialEntries={['/foo/hello']}>
-          <Test mockAnalytics={mockAnalytics}>
-            <Switch>
-              <Route path="/foo/:bar?">
-                <Page />
-              </Route>
-            </Switch>
-          </Test>
-        </MemoryRouter>,
+        <RecoilRoot>
+          <MemoryRouter initialEntries={['/foo/hello']}>
+            <Test mockAnalytics={mockAnalytics}>
+              <Switch>
+                <Route path="/foo/:bar?">
+                  <Page />
+                </Route>
+              </Switch>
+            </Test>
+          </MemoryRouter>
+        </RecoilRoot>,
       );
 
       jest.advanceTimersByTime(400);
@@ -68,15 +71,17 @@ describe('Analytics', () => {
       const mockAnalytics = createMockAnalytics();
 
       render(
-        <MemoryRouter initialEntries={['/foo/hello']}>
-          <Test mockAnalytics={mockAnalytics}>
-            <Switch>
-              <Route path="/foo/:bar?">
-                <Page />
-              </Route>
-            </Switch>
-          </Test>
-        </MemoryRouter>,
+        <RecoilRoot>
+          <MemoryRouter initialEntries={['/foo/hello']}>
+            <Test mockAnalytics={mockAnalytics}>
+              <Switch>
+                <Route path="/foo/:bar?">
+                  <Page />
+                </Route>
+              </Switch>
+            </Test>
+          </MemoryRouter>
+        </RecoilRoot>,
       );
 
       const button = screen.getByRole('button');
@@ -103,17 +108,19 @@ describe('Analytics', () => {
       const overrideAnalytics = createMockAnalytics();
 
       render(
-        <MemoryRouter initialEntries={['/foo/hello']}>
-          <Test mockAnalytics={mockAnalytics}>
-            <Test mockAnalytics={overrideAnalytics}>
-              <Switch>
-                <Route path="/foo/:bar?">
-                  <Page />
-                </Route>
-              </Switch>
+        <RecoilRoot>
+          <MemoryRouter initialEntries={['/foo/hello']}>
+            <Test mockAnalytics={mockAnalytics}>
+              <Test mockAnalytics={overrideAnalytics}>
+                <Switch>
+                  <Route path="/foo/:bar?">
+                    <Page />
+                  </Route>
+                </Switch>
+              </Test>
             </Test>
-          </Test>
-        </MemoryRouter>,
+          </MemoryRouter>
+        </RecoilRoot>,
       );
 
       jest.advanceTimersByTime(400);

diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/analytics.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/analytics.tsx
@@ -1,5 +1,11 @@
-import {createContext, useCallback, useContext, useEffect, useMemo} from 'react';
+import {createContext, useCallback, useContext, useLayoutEffect} from 'react';
 import {useLocation, useRouteMatch} from 'react-router-dom';
+import {atom, useRecoilValue, useSetRecoilState} from 'recoil';
+
+export const currentPageAtom = atom<{path: string; specificPath: string}>({
+  key: 'currentPageAtom',
+  default: {path: '/', specificPath: '/'},
+});
 
 export interface GenericAnalytics {
   group?: (groupId: string, traits?: Record<string, any>) => void;
@@ -13,10 +19,7 @@ export const AnalyticsContext = createContext<GenericAnalytics>(undefined!);
 const PAGEVIEW_DELAY = 300;
 
 export const usePageContext = () => {
-  const match = useRouteMatch();
-  const {pathname: specificPath} = useLocation();
-  const {path} = match;
-  return useMemo(() => ({path, specificPath}), [path, specificPath]);
+  return useRecoilValue(currentPageAtom);
 };
 
 const useAnalytics = () => {
@@ -52,28 +55,35 @@ export const dummyAnalytics = () => ({
 
 export const useTrackPageView = () => {
   const analytics = useAnalytics();
-  const {path, specificPath} = usePageContext();
+  const match = useRouteMatch();
+  const {pathname: specificPath} = useLocation();
+  const {path} = match;
+
+  const setCurrentPage = useSetRecoilState(currentPageAtom);
 
-  useEffect(() => {
+  useLayoutEffect(() => {
     // Wait briefly to allow redirects.
     const timer = setTimeout(() => {
       analytics.page(path, specificPath);
     }, PAGEVIEW_DELAY);
+    setCurrentPage({path, specificPath});
 
     return () => {
       clearTimeout(timer);
     };
-  }, [analytics, path, specificPath]);
+  }, [analytics, path, setCurrentPage, specificPath]);
 };
 
 export const useTrackEvent = () => {
   const analytics = useAnalytics();
-  const pathValues = usePageContext();
+  const match = useRouteMatch();
+  const {pathname: specificPath} = useLocation();
+  const {path} = match;
 
   return useCallback(
     (eventName: string, properties?: Record<string, any>) => {
-      analytics.track(eventName, {...properties, ...pathValues});
+      analytics.track(eventName, {...properties, path, specificPath});
     },
-    [analytics, pathValues],
+    [analytics, path, specificPath],
   );
 };
diff --git a/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillPage.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillPage.test.tsx
@@ -1,6 +1,7 @@
 import {MockedProvider} from '@apollo/client/testing';
 import {getAllByText, getByText, render, screen, waitFor} from '@testing-library/react';
 import {MemoryRouter, Route} from 'react-router-dom';
+import {RecoilRoot} from 'recoil';
 
 import {AnalyticsContext} from '../../../app/analytics';
 import {
@@ -82,15 +83,17 @@ const mocks = [
 describe('BackfillPage', () => {
   it('renders the loading state', async () => {
     render(
-      <AnalyticsContext.Provider value={{page: () => {}} as any}>
-        <MemoryRouter initialEntries={[`/backfills/${mockBackfillId}`]}>
-          <Route path="/backfills/:backfillId">
-            <MockedProvider mocks={mocks}>
-              <BackfillPage />
-            </MockedProvider>
-          </Route>
-        </MemoryRouter>
-      </AnalyticsContext.Provider>,
+      <RecoilRoot>
+        <AnalyticsContext.Provider value={{page: () => {}} as any}>
+          <MemoryRouter initialEntries={[`/backfills/${mockBackfillId}`]}>
+            <Route path="/backfills/:backfillId">
+              <MockedProvider mocks={mocks}>
+                <BackfillPage />
+              </MockedProvider>
+            </Route>
+          </MemoryRouter>
+        </AnalyticsContext.Provider>
+      </RecoilRoot>,
     );
 
     expect(await screen.findByTestId('page-loading-indicator')).toBeInTheDocument();
@@ -116,31 +119,35 @@ describe('BackfillPage', () => {
     ];
 
     const {getByText} = render(
-      <AnalyticsContext.Provider value={{page: () => {}} as any}>
-        <MemoryRouter initialEntries={[`/backfills/${mockBackfillId}`]}>
-          <Route path="/backfills/:backfillId">
-            <MockedProvider mocks={errorMocks}>
-              <BackfillPage />
-            </MockedProvider>
-          </Route>
-        </MemoryRouter>
-      </AnalyticsContext.Provider>,
+      <RecoilRoot>
+        <AnalyticsContext.Provider value={{page: () => {}} as any}>
+          <MemoryRouter initialEntries={[`/backfills/${mockBackfillId}`]}>
+            <Route path="/backfills/:backfillId">
+              <MockedProvider mocks={errorMocks}>
+                <BackfillPage />
+              </MockedProvider>
+            </Route>
+          </MemoryRouter>
+        </AnalyticsContext.Provider>
+      </RecoilRoot>,
     );
 
     await waitFor(() => expect(getByText('An error occurred')).toBeVisible());
   });
 
   it('renders the loaded state', async () => {
     render(
-      <AnalyticsContext.Provider value={{page: () => {}} as any}>
-        <MemoryRouter initialEntries={[`/backfills/${mockBackfillId}`]}>
-          <Route path="/backfills/:backfillId">
-            <MockedProvider mocks={mocks}>
-              <BackfillPage />
-            </MockedProvider>
-          </Route>
-        </MemoryRouter>
-      </AnalyticsContext.Provider>,
+      <RecoilRoot>
+        <AnalyticsContext.Provider value={{page: () => {}} as any}>
+          <MemoryRouter initialEntries={[`/backfills/${mockBackfillId}`]}>
+            <Route path="/backfills/:backfillId">
+              <MockedProvider mocks={mocks}>
+                <BackfillPage />
+              </MockedProvider>
+            </Route>
+          </MemoryRouter>
+        </AnalyticsContext.Provider>
+      </RecoilRoot>,
     );
 
     // Check if the loaded content is displayed

diff --git a/js_modules/dagster-ui/packages/ui-core/src/testing/TestProvider.tsx b/js_modules/dagster-ui/packages/ui-core/src/testing/TestProvider.tsx
@@ -1,6 +1,7 @@
 import {loader} from 'graphql.macro';
 import * as React from 'react';
 import {MemoryRouter, MemoryRouterProps} from 'react-router-dom';
+import {RecoilRoot} from 'recoil';
 
 import {ApolloTestProps, ApolloTestProvider} from './ApolloTestProvider';
 import {AppContext, AppContextValue} from '../app/AppContext';
@@ -73,25 +74,27 @@ export const TestProvider = (props: Props) => {
   );
 
   return (
-    <AppContext.Provider value={{...testValue, ...appContextProps}}>
-      <WebSocketContext.Provider value={websocketValue}>
-        <PermissionsContext.Provider
-          value={{
-            unscopedPermissions: extractPermissions(permissions),
-            locationPermissions: {}, // Allow all permissions to fall back
-            loading: false,
-            rawUnscopedData: [],
-          }}
-        >
-          <AnalyticsContext.Provider value={analytics}>
-            <MemoryRouter {...routerProps}>
-              <ApolloTestProvider {...apolloProps} typeDefs={typeDefs as any}>
-                <WorkspaceProvider>{props.children}</WorkspaceProvider>
-              </ApolloTestProvider>
-            </MemoryRouter>
-          </AnalyticsContext.Provider>
-        </PermissionsContext.Provider>
-      </WebSocketContext.Provider>
-    </AppContext.Provider>
+    <RecoilRoot>
+      <AppContext.Provider value={{...testValue, ...appContextProps}}>
+        <WebSocketContext.Provider value={websocketValue}>
+          <PermissionsContext.Provider
+            value={{
+              unscopedPermissions: extractPermissions(permissions),
+              locationPermissions: {}, // Allow all permissions to fall back
+              loading: false,
+              rawUnscopedData: [],
+            }}
+          >
+            <AnalyticsContext.Provider value={analytics}>
+              <MemoryRouter {...routerProps}>
+                <ApolloTestProvider {...apolloProps} typeDefs={typeDefs as any}>
+                  <WorkspaceProvider>{props.children}</WorkspaceProvider>
+                </ApolloTestProvider>
+              </MemoryRouter>
+            </AnalyticsContext.Provider>
+          </PermissionsContext.Provider>
+        </WebSocketContext.Provider>
+      </AppContext.Provider>
+    </RecoilRoot>
   );
 };