Skip to content
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.

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

[Feature Request] Use typescript enum for IPC routes #283

Open
Freymaurer opened this issue Oct 9, 2024 · 2 comments
Open

[Feature Request] Use typescript enum for IPC routes #283

Freymaurer opened this issue Oct 9, 2024 · 2 comments
Assignees
Labels
Type: Feature Request This item is confirmed by the maintainers to be a request for a new feature

Comments

@Freymaurer
Copy link
Collaborator

I used IPC routes in the Swate view, which were renamed some time ago, without adapting the calls in the SwateView.

RequestPaths: async (selectDirectories: boolean) => {
let selection = null;
let options: Electron.OpenDialogOptions = {}
options.defaultPath = ArcControlService.props.arc_root!;
if (selectDirectories) {
selection = await window.ipc.invoke("LocalFileSystemService.selectAnyFolders")
} else {
selection = await window.ipc.invoke("LocalFileSystemService.selectAnyFiles")
}
if (selection === null || selection.length < 1) return;
if (selection[0].startsWith(ArcControlService.props.arc_root)) {
selection = selection.map((p: string) => p.replaceAll(ArcControlService.props.arc_root!, "."))
}
send(Msg.PathsToSwate, selection)
},

cdeec89#diff-06a1adc8d17d7cee6fb88a8d3554ec88abb50f54b3350dbfc35c2b84eb1ab1c5L231-L234

This lead to issues for our users: nfdi4plants/Swate#518

I propose we use a typescript enum for all future ipc calls. This would give us more security in finding such errors.

Example:

image
image

@github-actions github-actions bot added the Status: Needs Triage This item is up for investigation. label Oct 9, 2024
@JonasLukasczyk JonasLukasczyk added Type: Feature Request This item is confirmed by the maintainers to be a request for a new feature and removed Status: Needs Triage This item is up for investigation. labels Oct 9, 2024
@JonasLukasczyk JonasLukasczyk self-assigned this Oct 9, 2024
@JonasLukasczyk
Copy link
Collaborator

If we keep both the string and enums then this just results in code bloat, i.e., a lot of redundancy . We could switch to a pure enum based IPC interface but I would give this a low priority.

@Freymaurer
Copy link
Collaborator Author

The images above serve only as an example. All IPC routes should be replaced by this setup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Request This item is confirmed by the maintainers to be a request for a new feature
Projects
Status: No status
Development

No branches or pull requests

2 participants