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

Export ActionFailure to allow instanceof #12611

Closed
probablykasper opened this issue Aug 25, 2024 · 6 comments · Fixed by #12878
Closed

Export ActionFailure to allow instanceof #12611

probablykasper opened this issue Aug 25, 2024 · 6 comments · Fixed by #12878
Labels
feature / enhancement New feature or request

Comments

@probablykasper
Copy link

Describe the problem

I have some shared logic for parsing user input, but it's not that convenient to deal with success/error types.

Describe the proposed solution

ActionFailure is a class, but SvelteKit exposes it as an interface. It would be nice to simply return fail() and do instanceof ActionFailure:

function parse(value: T | null) {
	if (!value) {
		return fail(400, { message: 'Too high' })
	}
	// ...
	return value
}

// Action:
const result = parse('test')
if (result instanceof ActionFailure) {
	return result
}

Alternatives considered

  • Creating my own class
  • Returning a result type like { value: T, error: null } | { value: null, error: string }
  • Use the Error class message as an intermediate, if I only need an error string

Importance

nice to have

Additional Information

No response

@eltigerchino
Copy link
Member

eltigerchino commented Oct 23, 2024

Does adding a check such as below solve your issue?

if ('status' in result && typeof result.status === 'number' && result.status >= 400) {
  return result;
}

or simply throw the ActionFailure so the return type is always T

function parse(value: T | null) {
	if (!value) {
		throw fail(400, { message: 'Too high' })
	}
	// ...
	return value
}

// Action:
try {
  const result = parse('test')
} catch (error) {
  if ('status' in error) {
    return error as ActionFailure;
  }
}

Using instanceof to check the type comes with it's own set of problems in monorepo setups that may have multiple SvelteKits installed and therefore multiple ActionFailure class instances. See #12597

@probablykasper
Copy link
Author

No, I don't feel like those are any simpler than the workarounds I mentioned. The second is also not type safe so I would highly discourage using that

@dummdidumm
Copy link
Member

We already have isError etc methods exposed from @sveltejs/kit - we should just add isActionFailure there

@dummdidumm dummdidumm added feature / enhancement New feature or request and removed awaiting submitter labels Oct 25, 2024
@probablykasper
Copy link
Author

Isn't it just as convenient and more performant to call instanceof directly?

@eltigerchino
Copy link
Member

eltigerchino commented Oct 25, 2024

We already have isError etc methods exposed from @sveltejs/kit - we should just add isActionFailure there

I don't think we do 🤔 but for some reason that sounds really familiar to me.

isError is not found

@dummdidumm
Copy link
Member

dummdidumm commented Oct 25, 2024

oops it's called isHttpError

Isn't it just as convenient and more performant to call instanceof directly?

I don't think performance matters here. What matters more is consistency, and we decided a while ago we want this to exposed via functions (gives more flexibility for the underlying implementation) and so it makes sense to keep it that way for new additions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants