Skip to content

PMM-14655 Add listeners for settings change and service added #4858

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.

Sign up for GitHub

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
matejkubinec merged 8 commits into from
Jan 14, 2026
+73 −1
Merged

PMM-14655 Add listeners for settings change and service added #4858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9917c4d
PMM-14655 Add listeners for settings change and service added
matejkubinec Dec 19, 2025
4849160
PMM-14655 Refactor refetching
matejkubinec Dec 19, 2025
d237ebe
PMM-14655 Fix lint
matejkubinec Dec 19, 2025
b77da3d
PMM-14655 Clear listeners on unregister
matejkubinec Jan 5, 2026
688281f
Merge branch 'v3' of github.com:percona/pmm into PMM-14655-ui-syncing
matejkubinec Jan 5, 2026
47645b2
Merge branch 'v3' of github.com:percona/pmm into PMM-14655-ui-syncing
matejkubinec Jan 13, 2026
2efd0ef
PMM-14655 Handle service deleted event
matejkubinec Jan 13, 2026
ae1c7c6
Merge branch 'v3' into PMM-14655-ui-syncing
matejkubinec Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions ui/apps/pmm-compat/src/compat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { adjustToolbar } from 'compat/toolbar';
import { isWithinIframe, getLinkWithVariables } from 'lib/utils';
import { documentTitleObserver } from 'lib/utils/document';
import { isFirstLogin, updateIsFirstLogin } from 'lib/utils/login';
import { ServiceAddedEvent, ServiceDeletedEvent, SettingsUpdatedEvent } from 'lib/events';

export const initialize = () => {
// If Grafana is opened outside of iframe (or on login), redirect to PMM UI
Expand Down Expand Up @@ -144,4 +145,22 @@ export const initialize = () => {
});
},
});

getAppEvents().subscribe(SettingsUpdatedEvent, () => {
messenger.sendMessage({
type: 'SETTINGS_CHANGED',
});
});

getAppEvents().subscribe(ServiceAddedEvent, () => {
messenger.sendMessage({
type: 'SERVICE_ADDED',
});
});

getAppEvents().subscribe(ServiceDeletedEvent, () => {
messenger.sendMessage({
type: 'SERVICE_DELETED',
});
});
};
18 changes: 18 additions & 0 deletions ui/apps/pmm-compat/src/lib/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @fileoverview
* Same events as in public/app/percona/shared/core/events.ts in grafana repository
*/

import { BusEventBase } from '@grafana/data';

export class SettingsUpdatedEvent extends BusEventBase {
static type = 'settings-updated-event';
}

export class ServiceAddedEvent extends BusEventBase {
static type = 'service-added-event';
}

export class ServiceDeletedEvent extends BusEventBase {
static type = 'service-deleted-event';
}
26 changes: 26 additions & 0 deletions ui/apps/pmm/src/contexts/grafana/grafana.provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { useKioskMode } from 'hooks/utils/useKioskMode';
import { useColorMode } from 'hooks/theme';
import { getLocationUrl } from './grafana.utils';
import messenger from 'lib/messenger';
import { useSettings } from 'hooks/api/useSettings';
import { useServiceTypes } from 'hooks/api/useServices';

/** Guard DOM usage. */
const isBrowser = () =>
Expand All @@ -34,6 +36,13 @@ export const GrafanaProvider: FC<PropsWithChildren> = ({ children }) => {
const location = useLocation();
const navigate = useNavigate();

const { refetch: refetchSettings } = useSettings({
enabled: false,
});
Copy link
Contributor

@dmitri-saricev-3pillargloball dmitri-saricev-3pillargloball Dec 23, 2025

Choose a reason for hiding this comment

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

Quick question about initial data loading

I see you're creating these queries with enabled: false:

const { refetch: refetchSettings } = useSettings({
enabled: false,
});

Just want to make sure I understand - are these queries being run somewhere else on initial load? If not, the UI might not have this data until the first event comes in. Might be worth a quick double-check, or if it's intentional, maybe add a comment so future devs know what's up ))))

Copy link
Contributor Author

@matejkubinec matejkubinec Jan 5, 2026

Choose a reason for hiding this comment

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

The initial fetch of settings is in SettingsProvider. This is just used for refetching when triggered from the Grafana side.

const { refetch: refetchServiceTypes } = useServiceTypes({
enabled: false,
});

Copy link
Contributor

@dmitri-saricev-3pillargloball dmitri-saricev-3pillargloball Dec 23, 2025

Choose a reason for hiding this comment

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

The main thing I noticed - listener cleanup, the listeners are being added, but I don't see them getting removed:

messenger.addListener({
type: 'SETTINGS_CHANGED',
onMessage: () => refetchSettings(),
});

This could cause a memory leak if the component remounts (like during navigation) - you'd keep stacking up listeners. The fix is pretty simple, though, just need to return a cleanup function:

useEffect(() => {
const settingsListener = messenger.addListener({
type: 'SETTINGS_CHANGED',
onMessage: () => refetchSettings(),
});

const serviceListener = messenger.addListener({
  type: 'SERVICE_ADDED',
  onMessage: () => refetchServiceTypes(),
});

return () => {
  messenger.removeListener(settingsListener);
  messenger.removeListener(serviceListener);
};

}, [refetchSettings, refetchServiceTypes]);

Copy link
Contributor Author

@matejkubinec matejkubinec Jan 5, 2026

Choose a reason for hiding this comment

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

Good catch, I've updated the unregister method in messenger to clear the listeners.

const src = location.pathname.replace(PMM_NEW_NAV_PATH, '');
const isGrafanaPage = src.startsWith(GRAFANA_SUB_PATH);

Expand Down Expand Up @@ -95,10 +104,27 @@ export const GrafanaProvider: FC<PropsWithChildren> = ({ children }) => {
},
});

messenger.addListener({
type: 'SETTINGS_CHANGED',
onMessage: () => refetchSettings(),
});

Copy link
Contributor

@dmitri-saricev-3pillargloball dmitri-saricev-3pillargloball Dec 23, 2025

Choose a reason for hiding this comment

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

A few other things to consider (not blockers, just ideas):

Error handling on refetch:
Right now if the refetch fails, it'll be silent. Maybe worth catching errors and showing a toast or logging them? Totally up to you though:

onMessage: async () => {
try {
await refetchSettings();
} catch (error) {
console.error('Failed to refetch settings:', error);
}
}

Copy link
Contributor Author

@matejkubinec matejkubinec Jan 5, 2026

Choose a reason for hiding this comment

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

I don't think there's a need to notify the user about it failing.

messenger.addListener({
type: 'SERVICE_ADDED',
onMessage: () => refetchServiceTypes(),
});

messenger.addListener({
type: 'SERVICE_DELETED',
onMessage: () => refetchServiceTypes(),
});

// Cleanup once provider unmounts
return () => {
messenger.unregister();
};

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isLoaded, setFromGrafana, navigate]);

// -------- OUTGOING TO GRAFANA --------
Expand Down
1 change: 1 addition & 0 deletions ui/apps/pmm/src/contexts/navigation/navigation.provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { useHaInfo } from 'hooks/api/useHA';
export const NavigationProvider: FC<PropsWithChildren> = ({ children }) => {
const { user } = useUser();
const { data: serviceTypes } = useServiceTypes({
enabled: !!user,
refetchInterval: INTERVALS_MS.SERVICE_TYPES,
});
const { settings } = useSettings();
Expand Down
1 change: 1 addition & 0 deletions ui/packages/shared/src/messenger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export class CrossFrameMessenger {
}

unregister() {
this.listeners = [];
this.window.removeEventListener('message', this.eventListener);
}

Expand Down
9 changes: 8 additions & 1 deletion ui/packages/shared/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ export type MessageType =
| 'GRAFANA_READY'
| 'DOCUMENT_TITLE_CHANGE'
| 'GRAFANA_THEME_CHANGED'
| 'CHANGE_THEME';
| 'CHANGE_THEME'
| 'SETTINGS_CHANGED'
| 'SERVICE_ADDED'
| 'SERVICE_DELETED';

export type LocationState = { fromGrafana?: boolean } | null;

Expand Down Expand Up @@ -55,3 +58,7 @@ export type ChangeThemeMessage = Message<
theme: ColorMode;
}
>;

export type SettingsChangedMessage = Message<'SETTINGS_CHANGED'>;

export type ServiceAddedMessage = Message<'SERVICE_ADDED'>;
Loading