Skip to content

bug: Module name discrepancy can lead to wrong expected view name #11

@thepeoplesbourgeois

Description

@thepeoplesbourgeois

tl;dr, I've got a pull request to make view_module an overridable function for apps that don't follow the de facto naming convention for LiveView modules. All unit tests pass. Thanks for building this 👍 🚀

First off, I want to express my gratitude to you for making this package. The controller paradigm is so helpful in keeping code well organized, especially (in my opinion) compared to the standard application structure of Phoenix Live View.

I'm incorporating live_controller into my Phoenix app now, and noticed that my app's structure surfaced a small usability issue for Phoenix apps with non-standard module namespaces. Specifically, I have made Live a namespace under my app's web namespace (as in PhoenixAppWeb.Live.{...}) to group together all of the common LiveView functionality.

As such, instead of e.g. PhoenixAppWeb.UserLive.Index and PhoenixAppWeb.UserLive.Show, or PhoenixAppWeb.UserLiveController modules, my app has PhoenixAppWeb.Live.User.Index, ...Web.Live.User.Show, and ...Web.Live.UserController modules. When determining its default view names, phoenix_live_controller anticipates that all live controllers have names that end with either Live or LiveController. Since mine don't, an exception was raised when I first added phoenix_live_controller, because ...Web.Live.UserController couldn't find a callable ...Web.Live.UserControllerView.render/2.

I forked this repository and added an overridable view_module function to the main module to fix this issue, as well as updated the String.replace/3 call to behave as expected when the controller module doesn't end with LiveController but rather just Controller. While implementing this, I noticed that live_controller's render function is also declared as overridable, and I considered just doing that in my app. Ultimately, I decided it would be better to add the overridable view_module/0 function, as overriding render would imply that some amount of work beyond changing the view's expected name would be necessary to properly render output for a controller action.

I've already got a pull request implementing this feature ready for you to review. It includes an update to the lockfile with the latest versions of all of live_controller's dependencies (your own ex_check among them) as well as a change to one of the unit tests to clear an unused variable warning from test runs. I hope you decide to merge my work.

Thank you again,
thepeoplesbourgeois

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions