Skip to content

Conversation

@sniok
Copy link
Contributor

@sniok sniok commented Nov 24, 2025

I've noticed that the details view still uses websocket multiplexer, which we disabled a while ago due to issues.

This PR reverts back to normal direct websocket connection,
Also moves multiplexer to a separate file so it's clearer (it was mixed with default websockets and probably that's why we forgot to disable it for the useKubeObject hook)

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 24, 2025
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 24, 2025
@sniok sniok changed the base branch from remove-multiplexer-from-details to main November 24, 2025 21:07
@sniok sniok force-pushed the remove-multiplexer-from-details branch from 052f436 to 972f869 Compare November 24, 2025 21:08
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sniok

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 24, 2025
@illume
Copy link
Contributor

illume commented Nov 26, 2025

Can you make it not hard to enable again?

@sniok sniok force-pushed the remove-multiplexer-from-details branch from 972f869 to 4190e6b Compare November 26, 2025 18:07
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 26, 2025
Comment on lines +174 to +194
// Breaking rules of hooks here a little but
// getWebsocketMultiplexerEnabled is a feature toggle
// and not a variable so this `if` should never change during runtime
if (getWebsocketMultiplexerEnabled()) {
useWebSocket<KubeListUpdateEvent<K>>({
url: () =>
makeUrl([KubeObjectEndpoint.toUrl(endpoint!)], {
...cleanedUpQueryParams,
watch: 1,
fieldSelector: `metadata.name=${name}`,
}),
enabled: !!endpoint && !!data,
cluster,
onMessage(update: KubeListUpdateEvent<K>) {
if (update.type !== 'ADDED' && update.object) {
client.setQueryData(queryKey, new kubeObjectClass(update.object));
}
},
});
} else {
useWebSockets({
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this violate the no-conditional hooks rule?

Copy link
Contributor Author

@sniok sniok Nov 26, 2025

Choose a reason for hiding this comment

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

kind of but not really, I explained it in the comment. Think of it as a compile time 'if', it will never change during the runtime

@sniok sniok force-pushed the remove-multiplexer-from-details branch from 4190e6b to f0d5d10 Compare November 26, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants