notify-rs/notify

Proposal: Change the serialization format of events

dfaust opened this issue · 2 comments

This relates to the notify integration into Tauri. With Tauri, developers can interact with notify using JavaScript. This means that they interface with events the way they are serialized using serde.

Currently a serialized event looks like this:

{"type":{"access":{"kind":"close","mode":"write"}},"paths":[],"attrs":{}}

This makes working with events in JavaScript a bit awkward. You have to test for event kinds individually and take into account that it may be a string or object:

let kind;
let mode;
// The two checks for `any` and `other` must be done first,
// otherwise TypeScript throws an error
if (event.type === "any") {
} else if (event.type === "other") {
} else if ("access" in event.type) {
    kind = event.type.access.kind;
    switch (event.type.access.kind) {
        case "any": {
            break;
        }
        case "close": {
            mode = event.type.access.mode; // "any" | "other" | "execute" | "read" | "write"
            break;
        }
        case "open": {
            mode = event.type.access.mode; // "any" | "other" | "execute" | "read" | "write"
            break;
        }
        case "other": {
            break;
        }
    }
} else if ("create" in event.type) {
    kind = event.type.create.kind;
} else if ("modify" in event.type) {
    kind = event.type.modify.kind;
    switch (event.type.modify.kind) {
        case "any": {
            break;
        }
        case "data": {
            mode = event.type.modify.mode; // "any" | "other" | "size" | "content"
            break;
        }
        case "metadata": {
            mode = event.type.modify.mode; // "any" | "other" | "access-time" | "write-time" | "permissions" | "ownership" | "extended"
            break;
        }
        case "name": {
            mode = event.type.modify.mode; // "any" | "other" | "to" | "from" | "both"
            break;
        }
        case "other": {
            break;
        }
    }
} else if ("remove" in event.type) {
    kind = event.type.remove.kind;
}

It would be more intuitive if we would flatten the event kind:

{"type":"access","kind":"close","mode":"write","paths":[],"attrs":{}}

Using it in JavaScript would look like:

let kind;
let mode;
switch (event.type) {
    case "any": {
        break;
    }
    case "access": {
        kind = event.kind;
        switch (event.kind) {
            case "any": {
                break;
            }
            case "close": {
                mode = event.mode; // "any" | "other" | "execute" | "read" | "write"
                break;
            }
            case "open": {
                mode = event.mode; // "any" | "other" | "execute" | "read" | "write"
                break;
            }
            case "other": {
                break;
            }
        }
        break;
    }
    case "create": {
        kind = event.kind;
        break;
    }
    case "modify": {
        kind = event.kind;
        switch (event.kind) {
            case "any": {
                break;
            }
            case "data": {
                mode = event.mode; // "any" | "other" | "size" | "content"
                break;
            }
            case "metadata": {
                mode = event.mode; // "any" | "other" | "access-time" | "write-time" | "permissions" | "ownership" | "extended"
                break;
            }
            case "name": {
                mode = event.mode; // "any" | "other" | "to" | "from" | "both"
                break;
            }
            case "other": {
                break;
            }
        }
        break;
    }
    case "remove": {
        kind = event.kind;
        break;
    }
    case "other": {
        break;
    }
}

Keeping it as it is would be fine, too. But I thought that very few people would be using the serialization feature anyway.
What do you think? Am I overthinking this?

@passcod I saw your comment in #487. Would such a change be a problem for you?

I don't have any real stakes in this. Flattening looks fine, but I don't want to be the only voice on this.

The format is part of watchexec's public API. So long as no information is lost such that I can reconstruct it to maintain compat, I'm fine with it.