codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.4k stars 1.9k forks source link

Bug: View cell cannot locate the auto-generated view file #7391

Closed sammyskills closed 1 year ago

sammyskills commented 1 year ago

PHP Version

7.4

CodeIgniter4 Version

4.3.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MariaDB 10.4.18

What happened?

When I generate a cell via the command php spark make:cell MyMagicCell, the command also generates a view file my_magic_cell.php which is normal according to the docs, but when I try to use the cell in a view file like so:

<?= view_cell('MyMagicCell::render') ?>

I get the following error: Error Exception: include(C:\xampp7427\htdocs\pbo\app\Cells\my_magic.php): failed to open stream: No such file or directory.

Upon troubleshooting, I found out that the _cell is actually removed here: https://github.com/codeigniter4/CodeIgniter4/blob/775a99e6518070dfc05deda8c1f843d08655acf2/system/View/Cells/Cell.php#L77 which conflicts with what is stated in the docs.

Controlled Cells have two primary goals: to make it as fast as possible to build the cell, and provide additional logic and flexibility to your views, if they need it. The class must extend CodeIgniter\View\Cells\Cell. They should have a view file in the same folder. By convention the class name should be PascalCase and the view should be the snake_cased version of the class name. So, for example, if you have a MyCell class, the view file should be my_cell.php.

One of two things can be done to resolve this, of which I can send a PR to fix it, but I need to know the direction that should be followed:

Steps to Reproduce

  1. Generate a cell: php spark make:cell MyAwesomeCell or php spark make:cell MyAwesome.
  2. Try to view the cell in any of your view files using view_cell('MyAwesomeCell::render') or view_cell('MyAwesome::render).

Expected Output

I expected that the cell's view file would be located without any error.

Anything else?

No response

kenjis commented 1 year ago
$ php spark make:cell MyAwesomeCell

CodeIgniter v4.3.3 Command Line Tool - Server Time: 2023-03-29 22:41:51 UTC+00:00

File created: APPPATH/Cells/MyAwesomeCell.php

File created: APPPATH/Cells/my_awesome_cell.php

$ php spark make:cell MyAwesome

CodeIgniter v4.3.3 Command Line Tool - Server Time: 2023-03-29 22:41:58 UTC+00:00

File created: APPPATH/Cells/MyAwesome.php

File exists: "APPPATH/Cells/my_awesome_cell.php"
$ php spark make:cell MyAwesome

CodeIgniter v4.3.3 Command Line Tool - Server Time: 2023-03-29 22:43:04 UTC+00:00

File created: APPPATH/Cells/MyAwesome.php

File created: APPPATH/Cells/my_awesome_cell.php
kenjis commented 1 year ago

I don't know why _cell is removed. This is fine: MyAwesomeCell → my_awesome_cell.php MyAwesome → my_awesome.php

And make:cell is something wrong? Why does it add _cell?

kenjis commented 1 year ago

Basically, the document is the specification. However, it does not mean that the documentation is free of errors. If implementation according to the document causes inconsistencies, the implementation may be modified.

In this case, the documentation about _cell in the filename is this only?

By convention the class name should be PascalCase and the view should be the snake_cased version of the class name. So, for example, if you have a MyCell class, the view file should be my_cell.php. https://codeigniter4.github.io/CodeIgniter4/outgoing/view_cells.html#controlled-cells

sammyskills commented 1 year ago

Another thing I noticed is that the file naming structure of the tests files follows the code implementation, instead of what is seen in the docs. See tests/_support/View/Cells.

For instance, there's a class named AdditionCell and a corresponding view named addition, and so on.

sammyskills commented 1 year ago

If we are to follow the document specification, that means the following will need to be modified:

This might be a breaking change, what do you think?

kenjis commented 1 year ago

I think it is better to fix these as bugs.

It is clearly described:

if you have a MyCell class, the view file should be my_cell.php.

and if _cell is removed, the following two classes use the same file.

MyAwesomeCell → my_awesome.php
MyAwesome → my_awesome.php

But it is not good that all existing apps will break, so it seems better to fall back to the file without _cell.

MyAwesomeCell → my_awesome_cell.php (if not found) → my_awesome.php

@lonnieezell Any opinion?

lonnieezell commented 1 year ago

Definitely a bug. I don't have a strong opinion, but I think falling back to the way without _cell works as a safety. I think it makes the most sense to append Cell onto the class name and have the view without, personally.

sammyskills commented 1 year ago

If Cell is appended to the class name, does this not defeat the essence of having the --suffix option for the make:cell command?

Some devs prefer to create the class name without the suffix. Following this suggestion might make the process too strict, IMO.

kenjis commented 1 year ago

it makes the most sense to append Cell onto the class name and have the view without, personally.

So the following implementation seems to be okay. It follows the current docs. It is not a breaking change. If a dev follows the above convention, it works.

  1. MyAwesomeCell → my_awesome_cell.php
  2. (if not found) → my_awesome.php
sammyskills commented 1 year ago

I sent a PR #7392