-
Notifications
You must be signed in to change notification settings - Fork 77
Update inertia generator #281
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
Conversation
90d2058 to
f08e8ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes support for Svelte 4 and updates the Inertia install generator with new defaults, including improved TypeScript support, updated page file structure, new type definitions, and modernized Inertia configuration.
Key changes:
- Removes all Svelte 4 templates, tests, and detection logic
- Updates example page paths from
InertiaExample.{ext}toinertia_example/index.{ext} - Adds TypeScript type definitions (vite-env.d.ts, globals.d.ts, index.ts) for all frameworks
- Introduces InertiaController base class with shared flash data
- Updates Inertia configuration with new defaults (future flags, form options, error handling)
Reviewed Changes
Copilot reviewed 67 out of 75 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/generators/install/install_generator_spec.rb | Removes svelte4 tests, updates file path expectations and import assertions |
| spec/generators/generators_helper_spec.rb | Removes svelte4 framework detection tests |
| lib/inertia_rails/generators/helper.rb | Simplifies framework detection to return 'svelte' for all Svelte versions |
| lib/inertia_rails/generators/controller_template_base.rb | Removes svelte4 case from extension logic |
| lib/generators/inertia_tw_templates/scaffold/templates/svelte4/* | Removes all Svelte 4 scaffold templates |
| lib/generators/inertia_templates/scaffold/templates/svelte4/* | Removes all Svelte 4 base templates |
| lib/generators/inertia/install/templates/svelte4/* | Removes all Svelte 4 install templates |
| lib/generators/inertia/install/templates/{framework}/types/* | Adds TypeScript type definitions for React, Vue, and Svelte |
| lib/generators/inertia/install/templates/{framework}/inertia.* | Updates entrypoint files with new defaults and error handling |
| lib/generators/inertia/install/templates/{framework}/InertiaExample.* | Updates example component paths and imports |
| lib/generators/inertia/install/templates/tailwind/application.css | Changes quote style from double to single quotes |
| lib/generators/inertia/install/templates/inertia_controller.rb | Adds new base controller with shared flash data |
| lib/generators/inertia/install/templates/controller.rb | Updates to use InertiaController and simplified render syntax |
| lib/generators/inertia/install/js_package_manager.rb | Renames internal method from package_manager to name |
| lib/generators/inertia/install/install_generator.rb | Updates generator logic for new file structure and adds bin/setup integration |
| lib/generators/inertia/install/frameworks.yml | Removes svelte4 configuration, updates file paths |
| docs/guide/server-side-setup.md | Updates documentation to remove Svelte 4 references |
| bin/generate_scaffold_example | Removes svelte4 option |
| .github/workflows/generators.yml | Removes svelte4 from test matrix, updates Ruby version to 3.4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| console.error( | ||
| "Missing root element.\n\n" + | ||
| "If you see this error, it probably means you loaded Inertia.js on non-Inertia pages.\n" + | ||
| 'Consider moving <%= vite_typescript_tag "inertia" %> to the Inertia-specific layout instead.', |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the error message template, vite_typescript_tag should be vite_javascript_tag since this is the JavaScript (not TypeScript) version of the file. The TypeScript version on line 59 correctly uses vite_typescript_tag.
| 'Consider moving <%%= vite_javascript_tag "inertia" %> to the Inertia-specific layout instead.', | ||
| "Missing root element.\n\n" + | ||
| "If you see this error, it probably means you load Inertia.js on non-Inertia pages.\n" + | ||
| 'Consider moving <%= vite_typescript_tag "inertia" %> to the Inertia-specific layout instead.', |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the error message template, vite_typescript_tag should be vite_javascript_tag since this is the JavaScript (not TypeScript) version of the file. The TypeScript version correctly uses vite_typescript_tag on line 35.
| 'Consider moving <%= vite_typescript_tag "inertia" %> to the Inertia-specific layout instead.', | |
| 'Consider moving <%= vite_javascript_tag "inertia" %> to the Inertia-specific layout instead.', |
| console.error( | ||
| "Missing root element.\n\n" + | ||
| "If you see this error, it probably means you loaded Inertia.js on non-Inertia pages.\n" + | ||
| 'Consider moving <%= vite_typescript_tag "inertia.jsx" %> to the Inertia-specific layout instead.', |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the error message template, vite_typescript_tag should be vite_javascript_tag since this is the JSX (JavaScript) version of the file. The TSX version correctly uses vite_typescript_tag on line 62 of inertia.tsx.
| 'Consider moving <%= vite_typescript_tag "inertia.jsx" %> to the Inertia-specific layout instead.', | |
| 'Consider moving <%= vite_javascript_tag "inertia.jsx" %> to the Inertia-specific layout instead.', |
| insert_into_file setup_file, " #{cmd}\n", | ||
| after: /system\('bundle check'\) \|\| system!\('bundle install'\)\n/ | ||
| else | ||
| say_status "Couldn't add `#{cmd}` script to bin/setup, add it manually", :red |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable cmd is referenced in the error message but is never defined in the scope. It should be defined before the conditional check, e.g., cmd = "#{pm_name} install" before line 311.
| render inertia: 'InertiaExample', props: { | ||
| name: params.fetch(:name, 'World'), | ||
| } | ||
| render inertia: { name: params.fetch(:name, 'World') } |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The render inertia: syntax is incorrect. It should be render inertia: 'inertia_example/index', props: { ... }. The current syntax render inertia: { name: ... } is missing the component name and will not work properly.
| render inertia: { name: params.fetch(:name, 'World') } | |
| render inertia: 'inertia_example/index', props: { name: params.fetch(:name, 'World') } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
little does copilot know, this was part of the goal!
f08e8ca to
17f2d7f
Compare
defd8c5 to
e77e236
Compare
New defaults for install generator:
InertiaControllercontroller