Skip to content

refactor(frontend): drop watchjoin#2925

Merged
talos-bot merged 2 commits into
siderolabs:mainfrom
Slessi:drop-watch-join
Jun 3, 2026
Merged

refactor(frontend): drop watchjoin#2925
talos-bot merged 2 commits into
siderolabs:mainfrom
Slessi:drop-watch-join

Conversation

@Slessi

@Slessi Slessi commented Jun 2, 2026

Copy link
Copy Markdown
Member

Drop WatchJoin and related code and replace with manual merging as needed. The complexity it brings is easily solved with array maps, and is rarely needed. As a nice quick side benefit, TList items is typeable now and use cases have been typed.

Related #1471

Comment on lines +58 to +59
const v1node = v1nodes.value.find((n) => n.metadata.name === itemID)
const talosMember = talosMembers.value.find((m) => m.metadata.id === itemID)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The complexity of that function is rather big now as we call find twice here.
If we go this path I think we should build a map where n.metadata.name/m.metadata.id is the key and the value is the resource. Then look up the extra data in that map.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's absolutely worse, and a map is absolutely better, but will we be dealing with datasets large enough to care? If every list had 1k items it would probably take like 5-10ms to iterate this at the most.

If we are dealing with large data like that then sure, but if not probably not important. Mostly wondering cause it would be good for me to know to expect large datasets like that in places like this.

(Keeping in mind there is also pagination here, so at most, it is going to be 100 items -> 200 finds, if page is set to 100, which is pretty cheap)

@Slessi Slessi Jun 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

in any case, added now, easy change 🚀

...m,
spec: {
...m.spec,
...machineConfigGenOptions.value.find((c) => c.metadata.id === m.metadata.id)?.spec,

@Unix4ever Unix4ever Jun 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same. Lets not use find there.
Maybe we can have a helper function computedMap(items, idFunc).
Then:

const machineConfigGenOptsMap = computedMap(machineConfigGenOpts, (c) => c.metadata.id)
...
...machineConfigGenOptsMap.value[m.metadata.id]?.spec,

Slessi added 2 commits June 2, 2026 23:36
Drop WatchJoin and related code and replace with manual merging as needed. The complexity it brings is easily solved with array maps, and is rarely needed.

Signed-off-by: Edward Sammut Alessi <edward.sammutalessi@siderolabs.com>
Type TList items using generics and a fake prop

Signed-off-by: Edward Sammut Alessi <edward.sammutalessi@siderolabs.com>
@github-project-automation github-project-automation Bot moved this from In Review to Approved in Planning Jun 3, 2026
@Slessi

Slessi commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

/m

@talos-bot talos-bot merged commit 0610a40 into siderolabs:main Jun 3, 2026
35 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Done in Planning Jun 3, 2026
@Slessi Slessi deleted the drop-watch-join branch June 3, 2026 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants