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

Prevent needing to use Method enum with the Link component #1392

Merged
merged 5 commits into from
Jan 18, 2023

Conversation

jessarcher
Copy link
Member

This PR fixes an issue where TypeScript users now need to use the Method enum with the Link component.

Prior to 1.0, the Method enum was only needed when calling Inertia.visit() in TypeScript, but in most cases, people were probably using the shortcut methods that bypass the need to specify the method argument.

When the adapters were converted to TypeScript, their existing type definitions specified the method prop as a string, but this was not compatible with the stricter definition that already existed in core. Rather than loosening the definition in core, I opted to make the adapter definitions compatible. In hindsight, this creates an unnecessary burden on TypeScript users.

As suggested in #1384 (comment), this PR introduces a union type that offers the same type safety but without needing to import the type.

Removing the existing enum would be a breaking change, so this PR just deprecates it for now.

Fixes #1384

@jessarcher jessarcher marked this pull request as ready for review January 17, 2023 03:13
jessarcher and others added 4 commits January 17, 2023 21:58
@reinink reinink force-pushed the replace-method-enum branch from 7ca444e to a9171db Compare January 18, 2023 02:58
@reinink
Copy link
Member

reinink commented Jan 18, 2023

As discussed, decided to just change the existing Method enum to a new string based union type. Technically this type was never exported from any of the adapters, so this is considered private API, and not a breaking change.

@reinink reinink merged commit 471d177 into master Jan 18, 2023
@reinink reinink deleted the replace-method-enum branch January 18, 2023 03:57
@palypster
Copy link

palypster commented Feb 15, 2023

Well, except, it was exported (in @inertiajs/core) and thus it did break our apps :) Do you really find it as a private API? No worries, we easily fixed that, I'm just curious about your view on the "private API" part, so we know what to expect in the future. Awesome work guys, thank you! <3

@Sam-Apostel
Copy link

Agree with palypster, this was a breaking change. Could we patch this by exporting a Method object from core?
Could be removed in the next breaking version.

@alucic
Copy link

alucic commented Mar 24, 2023

This is a breaking change

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 with Link component together with "method"
6 participants