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

Typescript error with Link component together with "method" #1384

Closed
johan-paqt opened this issue Jan 16, 2023 · 3 comments · Fixed by #1392
Closed

Typescript error with Link component together with "method" #1384

johan-paqt opened this issue Jan 16, 2023 · 3 comments · Fixed by #1392
Assignees

Comments

@johan-paqt
Copy link

johan-paqt commented Jan 16, 2023

Version:

  • @inertiajs/vue3 version: 1.0.0
  • typescript version: 4.9.4

Describe the problem:

If you use a <Link like so:

<Link
    as="button"
    method="post"

It gives the following error:
Type '"post"' is not assignable to type 'Method | undefined'.

What does work, is using the Method enum like so:

import { Method } from '@inertiajs/core';

<Link
    as="button"
    :method="Method.POST"

So if the plain method="post" is no longer supported, it should be reflected in the docs.

Steps to reproduce:

See the description.

@johanmolen
Copy link

I think an union type would be better to use in this case, instead of an Enum.

@innocenzi
Copy link
Contributor

innocenzi commented Jan 16, 2023

Chiming in to confirm that TypeScript enums are 99.9% of the time a bad choice. Union types are almost always better, because they don't need a runtime, hence an import.

@jessarcher
Copy link
Member

Hey, thanks for reporting this.

I'm assuming this was already the case with Inertia.visit() (now router.visit()) prior to the 1.0 release. It probably didn't come up often as there are shorthand methods like post() that bypass the need to specify the method argument.

The adapters previously weren't written in TypeScript, so their type definitions weren't 100% compatible with the core package when converting them to TypeScript. In hindsight, it probably would have been better to change the core definition rather than the adapter definitions.

It's a bit late to remove the enum now, but I have created a #1392 that introduces a union type.

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 a pull request may close this issue.

4 participants