feat: Hot Reloading

[lonnieezell]

This adds a new "Hot Reload" feature to the debug toolbar. When active, it will scan the app directory for any changes to files and instruct the browser to reload if any changes were found.

I still need to write tests, but submitting it so that others can try it out first and see if you encounter any problems. I've seen at least one strange thing but unsure if it is just my setup or a more widespread problem.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[michalsn]

Looks good!

I gave it a try on:

• Firefox, Chrome, and Safari on macOS (with nginx)
• Firefox, Chrome, and Edge on Windows (with Apache)

Besides some general comments on the code, I noticed some visual issues when hot reloading is enabled:

hot-reload-visual-issue.mp4

Please take a look at the vertical scroll bar on the toolbar, which is moving while the icon is spinning.

[lonnieezell]

@michalsn Thanks for these comments! Good stuff, and exactly why I was wanting other eyes on this. I had missed the scrollbar wackiness. Maybe having the animation is more trouble than it is worth but I'll take another look at it.

[iRedds]

If this feature is associated with a tool bar, then it must be in the CodeIgniter\Debug namespace.
Should only be enabled if the toolbar is enabled.

[lonnieezell]

If this feature is associated with a tool bar, then it must be in the CodeIgniter\Debug namespace.

I disagree. This is no more a toolbar feature than any of the other info panes are associated with the toolbar. There is a button on the toolbar that starts and stops it, but that is as far as the association goes.

Should only be enabled if the toolbar is enabled.

I can operate without the toolbar if someone wanted to implement just a few lines of JS on their site. There's nothing that would stop it from working. However, in practice it does operate only when the toolbar is enabled, since it's currently the only way to start it. If the toolbar is disabled during while the hot reloader is active then the connection would be closed on page refresh.

[michalsn]

Thanks, @lonnieezell - now it's working great!

Although there is still a small problem with this icon. True, the slider does not appear now, but the icon moves slightly up and down. This is probably a problem with selecting the wrong element in the DOM. We should be working on the img element, not a. I have added a code suggestion to solve this.

[lonnieezell]

Why is it so difficult to get a PR to a place to make our 25 checks happy. This is so frustrating to work with anymore.

PHP Linter is failing due to stuff that has nothing to do with this PR. I rebased against the current 4.4 branch and they're still there.

I realized that I had put my CSS changes in the CSS file, forgetting that we had a SASS file in place, so I moved that change, ran dart sass locally as explained in our contributing docs, and all worked fine, but the tests are failing here.

Why are we being forced to add @extends / @implements into the docblocks? How is this helpful at all? I'm actually curious. I've been programming PHP for 15 years and never needed it. It seems pretty obvious by looking at the class declaration what it extends and implements. What is the benefit? And what is the right lines to use in the IteractorFilter because I obviously don't have a clue.

[lonnieezell]

@paulbalandan @kenjis What's the proper lines to have in the docblock here to make our static analysis happy?

[kenjis]

Something like this? But I don't know the exact types.

 * @template-extends RecursiveFilterIterator<TKey, TValue, TIterator>
 * @template-implements RecursiveIterator<TKey, TValue>

There is some question as to how useful this type information is, so we can suppress errors.

--- a/system/HotReloader/IteratorFilter.php
+++ b/system/HotReloader/IteratorFilter.php
@@ -17,8 +17,7 @@ use RecursiveIterator;
 /**
  * @internal
  *
- * @extends RecursiveFilterIterator<RecursiveIterator>
- * @implements RecursiveIterator<RecursiveIterator>
+ * @psalm-suppress MissingTemplateParam
  */
 final class IteratorFilter extends RecursiveFilterIterator implements RecursiveIterator
 {

[lonnieezell]

Thank you, that works.

[kenjis]

PHP Linter is failing due to stuff that has nothing to do with this PR. I rebased against the current 4.4 branch and they're still there.

Rebase and composer update will solve the error.

[kenjis]

Why are we being forced to add @extends / @implements into the docblocks? How is this helpful at all? I'm actually curious. I've been programming PHP for 15 years and never needed it. It seems pretty obvious by looking at the class declaration what it extends and implements. What is the benefit? And what is the right lines to use in the IteractorFilter because I obviously don't have a clue.

It is generics. It provide more type information that cannot express PHP syntax.
https://psalm.dev/docs/annotating_code/templated_annotations/

When removing all tags:

ERROR: MissingTemplateParam - system/HotReloader/IteratorFilter.php:19:13 - CodeIgniter\HotReloader\IteratorFilter has missing template params when extending RecursiveFilterIterator, expecting 3 (see https://psalm.dev/182)
final class IteratorFilter extends RecursiveFilterIterator implements RecursiveIterator


ERROR: MissingTemplateParam - system/HotReloader/IteratorFilter.php:19:71 - CodeIgniter\HotReloader\IteratorFilter has missing template params when extending RecursiveIterator, expecting 2 (see https://psalm.dev/182)
final class IteratorFilter extends RecursiveFilterIterator implements RecursiveIterator


ERROR: MissingTemplateParam - system/HotReloader/IteratorFilter.php:21:71 - CodeIgniter\HotReloader\IteratorFilter has missing template params when extending RecursiveIterator, expecting 2 (see https://psalm.dev/182)
final class IteratorFilter extends RecursiveFilterIterator implements RecursiveIterator


ERROR: MissingTemplateParam - system/HotReloader/IteratorFilter.php:32:13 - CodeIgniter\HotReloader\IteratorFilter has missing template params when extending RecursiveFilterIterator, expecting 3 (see https://psalm.dev/182)
final class IteratorFilter extends RecursiveFilterIterator implements RecursiveIterator

[kenjis]

What is the benefit?

Yeah, it is certainly too complicated to know what it is.

https://github.com/vimeo/psalm/blob/81946bbbc39b3e5c688815b58c905d7d8d64146e/stubs/CoreGenericIterators.phpstub#L679-L687

https://github.com/vimeo/psalm/blob/81946bbbc39b3e5c688815b58c905d7d8d64146e/stubs/CoreGenericIterators.phpstub#L67C1-L73

[kenjis]

I realized that I had put my CSS changes in the CSS file, forgetting that we had a SASS file in place, so I moved that change, ran dart sass locally as explained in our contributing docs, and all worked fine, but the tests are failing here.

Running sass will update toolbar.css. Did you update sass?

$ sass --no-source-map admin/css/debug-toolbar/toolbar.scss system/Debug/Toolbar/Views/toolbar.css

$ git diff
diff --git a/system/Debug/Toolbar/Views/toolbar.css b/system/Debug/Toolbar/Views/toolbar.css
index 60c3216bfc..38bab087ae 100644
--- a/system/Debug/Toolbar/Views/toolbar.css
+++ b/system/Debug/Toolbar/Views/toolbar.css
@@ -7,327 +7,321 @@
  * file that was distributed with this source code.
  */
 #debug-icon {
-    bottom: 0;
-    position: fixed;
-    right: 0;
-    z-index: 10000;
-    height: 36px;
-    width: 36px;
-    margin: 0px;
-    padding: 0px;
-    clear: both;
-    text-align: center;
+  bottom: 0;
+  position: fixed;
+  right: 0;
+  z-index: 10000;
+  height: 36px;

[lonnieezell]

@kenjis Thanks for your help. I was pretty frustrated last night. I feel like every time I have a contribution I spend as long trying to appease the tools as I do working on the feature itself. It always seems to happen that you fix one tooling issue and more crop up.

And, while I understand what generics are, my usage of them is limited to within Typescript when needed.

But I just have to stop and think that if I find this so challenging to work with at times, I can't imagine what non-core contributors feel when they want to contribute and how many PRs this whole process has stopped from happening.

[lonnieezell]

And it's all green!

[kenjis]

I tested this feature a little with spark serve.
It seems to work fine.

[michalsn]

🚀

[lonnieezell]

See - I make changes to some user guide pages and suddenly Rector has issues in the RequestTrait that it didn't previously have.

/sigh.

Hopefully we don't need to address that for this and can get this merged.

[datamweb]

I make changes to some user guide pages and suddenly Rector has issues in the RequestTrait that it didn't previously have.

This is not related to this PR.

[kenjis]

Don't worry. Rector check fails now. See #7606

[lonnieezell]

Thanks for merging!

[ALTITUDE-DEV-FR]

Excellent feature thanks !!!

[lonnieezell]

Glad you like it!

[ALTITUDE-DEV-FR]

Glad you like it!

i have small problem with this.
EventSource's response has a MIME type ("text/html") that is not "text/event-stream". Aborting the connection.