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

Optional input argument #4

Closed
probablykasper opened this issue Dec 2, 2022 · 13 comments
Closed

Optional input argument #4

probablykasper opened this issue Dec 2, 2022 · 13 comments

Comments

@probablykasper
Copy link
Contributor

Not sure how it would be done, but would be great to have the input be optional when a function has no argument. Currently you have to supply null. Setting the type to void might be an option.

@Brendonovich
Copy link
Collaborator

I don't think the solution you've suggested will work since you still need to pass an argument of type void (impossible), and it's similar to situations in rspc where a query/mutation has no argument. I think some changes to the codegen I have planned could alleviate this problem though - generating a new function per command rather than typedInvoke.

@oscartbeaumont
Copy link
Member

void wouldn't work but vardic args can be used for this similar to rspc and trpc.

@probablykasper
Copy link
Contributor Author

probablykasper commented Dec 3, 2022

Are you sure void wouldn't work? undefined is assignable to void. iirc it worked fine if I update the bindings

@Brendonovich
Copy link
Collaborator

Are you sure void wouldn't work? undefined is assignable to void. iirc it worked fine if I update the bindings

Yeah that would work, but you'd still have to provide an argument. We could just make the argument section optional entirely (which is less typesafe) or use the system I've been working on which exports discrete functions that match their corresponding commands.

@probablykasper
Copy link
Contributor Author

Yeah that would work, but you'd still have to provide an argument.

That doesn't seem to be how it works. If you see the PR I linked, it made trailing void parameters optional

I think having discrete functions sounds amazing though, would love to see that!

@Brendonovich
Copy link
Collaborator

Brendonovich commented Dec 16, 2022

This issue shouldn't apply with 9107481.

@probablykasper
Copy link
Contributor Author

That seems like it should work indeed, looks wonderful to use.

Now that I think about it though, the functions present a bit of a challenge for me. At the moment, I have a wrapper for invoking commands, so that I don't need a try-catch around every invoke call:

async function runCmd(cmd, input) {
  try {
    return await invoke(cmd, input)
  } catch (e) {
    invoke('error_popup', { msg: String(e) })
    throw e
  }
}

Not quite sure how I should deal with that one 🤔

@Brendonovich
Copy link
Collaborator

Brendonovich commented Dec 17, 2022

@probablykasper I can seen a few ways of doing this:
Perhaps the simplest is to manually catch the promise itself and give it a function that does what you need:

import { helloWorld, errorPopup } from "./bindings";

async function main() {
  const result = await helloWorld("Test").catch(rethrowWithPopup);
}

function rethrowWithPopup(e: any) {
  errorPopup(String(e));
  throw e;
}

Alternatively, let rethrowWithPopup do the try/catch for you:

import { helloWorld, errorPopup } from "./bindings"

async function main() {
  const result = await rethrowWithPopup(() => helloWorld("Test"));
}

async function rethrowWithPopup<T>(callback: () => Promise<T>) {
  try {
    return await callback();
  } catch (e: any) {
    errorPopup(String(e));
    throw e;
  }
}

If this is all too clunky, you could get a bit crazy and use a Proxy to rewrite the underlying logic:

// utils.ts
import * as commands from "./bindings";

export const commands = new Proxy({}, {
  get: (_, property) => async (...args) => {
    try {
      return await commands[string](...args)
    } catch (e) {
      commands.errorPopup(String(e));
      throw e
    }
  }
);

// main.ts
import { commands } from "./utils";

async function main() {
  commands.helloWorld("Test");
}

This approach isn't very TypeScript friendly though, you'd need a generic that can index the exports of the star import and idk how to do that.

@probablykasper
Copy link
Contributor Author

Yeah I think something like the the last option is what I'll need to look at - I don't want to risk forgetting to handle an error

@Brendonovich
Copy link
Collaborator

Brendonovich commented Dec 17, 2022

@probablykasper This seems to work great in TS:

// utils.ts
import * as c from "./bindings";

const commands = new Proxy({} as typeof c, {
  get:
    (_, property: string) =>
    async (...args: any[]) => {
      try {
        return await (c as any)[property](...args);
      } catch (e) {
        c.helloWorld(String(e));
        throw e;
      }
    },
});

// main.ts
import { commands } from "./utils";

async function main() {
  commands.helloWorld("Test");
}

I reckon we can close this issue now.

@probablykasper
Copy link
Contributor Author

Oh cool, I'll try it out! Thank you so much, didn't expect you to just do it all for me haha

@Brendonovich
Copy link
Collaborator

No worries, didn't wanna leave you without a proper solution before marking this as done haha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants