rails / tailwindcss-rails

MIT License
1.39k stars 171 forks source link

generator: sync erb template #357

Closed kinsomicrote closed 5 months ago

kinsomicrote commented 5 months ago

This updates the ERB template to be in sync with the template in rails/rails

Fixes #341.

kinsomicrote commented 5 months ago

Hello @flavorjones 👋

Any pointers on why the CI is failing? I ran the test suite locally, and they all passed.

javierjulio commented 5 months ago

@kinsomicrote based on the errors occurring for the Rails 6.1 build, it looks like model_resource_name and index_helper have a different method signature at that version. So using model_resource_name as an example, the v7.1 docs differ from the v6.1 docs.

kinsomicrote commented 5 months ago

Thank you, @javierjulio

Would it be better to leave it as it was? For example, using index_helper %>_path instead of index_helper(type: :path) %>?

javierjulio commented 5 months ago

@kinsomicrote you're welcome. That could work. In general though for both methods I don't know what the maintainers here would prefer. For example, they could suggest a Rails version conditional to account for the difference instead. I just wanted to help identify the reason for the failure.

kinsomicrote commented 5 months ago

@flavorjones tests are passing now. Let me know if it's good to go.

flavorjones commented 5 months ago

Quick note for posterity: the model_resource_name change is from rails/rails#43611 which first appeared in Rails 7.0 and fixed namespaced models.

Namespaced models (e.g. Admin::Monster) still don't work with this PR but that's probably OK as long as we have to support Rails 6.1

flavorjones commented 5 months ago

I've rebased and squashed

flavorjones commented 5 months ago

@kinsomicrote Thanks so much for doing this work!!!

I'm going to open a second PR with your changes using the Rails 7.0 API so we can have a separate conversation about dropping Rails 6.1 feature support.

Edit: that PR is #359.