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

fix: allow any action payload if action was never typed #5

Merged
merged 3 commits into from
Dec 14, 2021

Conversation

ivanhofer
Copy link
Contributor

closes #4

@ivanhofer
Copy link
Contributor Author

With this PR it is possible to pass any type and amount of arguments as long as the action function was never typed.
As soon as the action would be paginate(page: number) instead of paginate(), this would not work.

@kenkunz
Copy link
Owner

kenkunz commented Dec 14, 2021

Thanks again for the quick response and fix, @ivanhofer!

If I'm understanding this correctly, if someone is using vanilla JS (no explicit type annotations) but still relying on the type declarations for IDE feedback, they will always be able to use any argument arity and types in event invocations. When using TypeScript, if the action method signatures are explicitly defined, you will only be able to use the specified arity / types. Correct?

Assuming that's correct – I think this approach makes sense. Given the dynamic nature of JS and svelte-ism, and the way args can be forwarded to lifecycle events, it makes sense that the type validation would be less strict (allow for all possible argument passing scenarios) if the user has not explicitly defined the allowed args.

@kenkunz
Copy link
Owner

kenkunz commented Dec 14, 2021

After reviewing the code and tests, I believe there may be another small bug in the type defs.

An action method can return a string, symbol or undefined – e.g.,

const machine = fsm('foo', {
  foo: {
    returnsString() { return 'bar'; },
    returnsSymbol() { return Symbol.for('bar'); }
    returnsUndefined() {}
  }
});

…but the event invocations always return a string or symbol – because the event invocations return the current state (after any transitions resulting from the invocation), which can only be a string or symbol. So the below event invocations would return the values shown in the comments (assuming they are not invoked sequentially … just a single invocation after initializing machine):

machine.returnsString()    // => 'bar' (action caused a transition to 'bar')
machine.returnsSymbol()    // => Symbol.for('bar') (action caused a transition to Symbol.for('bar') )
machine.returnsUndefined() // => 'foo' (action returned undefined, so there was no transition)

Looking at our current type tests – it appears our type defs expect a void return value when the action returns undefined.

Sorry… I should have caught this in the previous PR. Hoping this is relatively straightforward to fix 🤞?

@ivanhofer
Copy link
Contributor Author

ivanhofer commented Dec 14, 2021

@kenkunz returnsUndefined() {} would be typed as void. You would need to write returnsReallyUndefined() { return undefined } to make the call signature return undefined.
In my opinion the types are correct as they are right now

@kenkunz
Copy link
Owner

kenkunz commented Dec 14, 2021

@kenkunz returnsUndefined() {} would be typed as void. You would need to write returnsReallyUndefined() { return undefined } to make the call signature return undefined.

I think the confusion here stems from literally "speaking two different languages" (JS vs TS).

In JS, void is not a type or even a value… it's an operator that always returns undefined:

(() => {})() === undefined;  // true
void 0 === undefined;        // true

It TS (of which I am admittedly a novice and heavily relying on your expertise), I gather that void has a different intent as a function return type than undefined.

Per the TS docs:

void represents the return value of functions which don’t return a value. It’s the inferred type any time a function doesn’t have any return statements, or doesn’t return any explicit value from those return statements:

In JavaScript, a function that doesn’t return any value will implicitly return the value undefined. However, void and undefined are not the same thing in TypeScript.

Contextual typing with a return type of void does not force functions to not return something. Another way to say this is a contextual function type with a void return type (type vf = () => void), when implemented, can return any other value, but it will be ignored.

Excuse my ignorance in these matters… TS is relatively new to me. I'll try to be clear in the future if I'm "speaking" in JS vs. TS. If I don't specify, you can assume I'm speaking JS.

Based on the last quoted paragraph above, I think you're correct in identifying void as a valid action function type – we expect actions to either return a valid state (string | symbol) … or if not, their return value is disregarded as irrelevant.

In contrast to action functions (as defined in the fsm states {...} arg), event invocations (calling a method on the returned state machine) always return a string | symbol.

@kenkunz
Copy link
Owner

kenkunz commented Dec 14, 2021

@ivanhofer your changes in 18fdc8f look good – I'm going to merge this and close the PR & issue.

Thanks again! Hopefully we won't see any other issues. I don't expect to make any API changes for a while – I'd like to see how this is used in the wild for a bit / what type of enhancement requests come in.

@kenkunz kenkunz merged commit e7c00ad into kenkunz:main Dec 14, 2021
@ivanhofer
Copy link
Contributor Author

@kenkunz and I didn't know it behaves like tis in JavaScript 😅
I exclusively use TypeScript since 2+ years now, so I might have forgotten how normal JavaScript behaves in certain situations.
Thanks for posting the link to StackOverflow 👍

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

Successfully merging this pull request may close these issues.

Typescript error on event invocation with argument
2 participants