Skip to content

Add patch for validation error types on inertia:install #219

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

Closed

Conversation

pricci1
Copy link

@pricci1 pricci1 commented Apr 14, 2025

This is part of #215

As commented in that PR, this changes add the suggested inertia-rails.d.ts type overrides to the inertia:install generator.

Considering that the code examples in the documentation and the generators pass the errors as arrays, maybe the global re-export should be part of the setup guide or even the inertia:install generator.

Also, as suggested by @skryukov, a link to the yet-to-be-merged cookbook is included in the inertia-rails.d.ts file.

In the docs' code examples and code scaffolded by `inertia:install`, the
Rails' validation errors object (array of strings) is passed as-is to
Inertia's rederer.

To be consistent, when using `inertia:install` task, the Typescript
definitions are patched.
@pricci1 pricci1 marked this pull request as ready for review April 14, 2025 16:04
@skryukov
Copy link
Contributor

Thanks for the PR @pricci1!

I believe @bknoles has some pushback on this approach—maybe we should include the inertia_errors helper instead to align with Inertia.js types. I also tend to do that in the ShadCN starter, for example.

Another issue with this approach is the fragile nature of these type patches—that's a major downside to this approach tbh 🤔

@bknoles wdyt?

@pricci1
Copy link
Author

pricci1 commented Apr 22, 2025

I agree that it is quite fragile and another thing to maintain.

Feel free to close!

@bknoles
Copy link
Collaborator

bknoles commented Apr 23, 2025

Yea, I like that we're documenting the "redefine the types" option but I don't want to bake it into the library. That said, I echo @skryukov to say thanks for the PR @pricci1 !

@bknoles bknoles closed this Apr 23, 2025
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.

3 participants