Improve visual for hot reload

matthewmueller commented 2 years ago

Lemme know when you want me to have a look at this @012e!

012e commented 2 years ago

@matthewmueller I think it's fine now

matthewmueller commented 2 years ago

Sorry for the delay @012e! Excited to try this. Will try this out very soon and get back to you. Thanks for your hard work!

matthewmueller commented 2 years ago

This is awesome @012e!! I'm quite impressed with how well this works, nice job!

Three tweaks I'd like to make to functionality before reviewing code and merging:

1. Improve the boot logging

Right now when you run bud -C example/basic run, you'll see the following:

$ bud -C example/basic run
| Listening on http://127.0.0.1:3000
|

Ideally it should be:

$ bud -C example/basic run
| Listening on http://127.0.0.1:3000

Then | Listening on http://127.0.0.1:3000 gets replaced by | Ready on http://127.0.0.1:3000 in 84ms (x4)

2. Should also log for changes to .svelte files

Right now it's missing logging when you change the views. It should also show | Ready on http://127.0.0.1:3000 in 15ms (x4)

3. The count just counts up?

It's a bit weird that the count only seems to increment. I had a look at what Vite does and it will reset if you adjust another file.

If I make 4 adjustments to the index.html, you'll see:

11:55:30 PM [vite] page reload index.html (x4)

Then if I make two adjustments to the main.ts file:

11:57:58 PM [vite] page reload src/main.ts (x2)

I'm honestly not sure if it's that useful to show the last modified file name. I'm tempted to suggest we just remove the counter.

What do you think?

012e commented 2 years ago

1. Improve the boot logging

I am thinking about this

On a normal, single file change, the counter
- Reset if failed to reload or user switched file (eg hello/controller.go to world/controller.go)
- Increase on success reloads
```
| Ready on http://127.0.0.1:3004 in 106ms
| Changed controller/controller.go (x29)
```

Multiple file changed (eg create a new route)

| Ready on http://127.0.0.1:3004 in 990ms
| Changed: 
|    controller/ok
|    controller/ok/controller.go
|    view/ok
|    view/ok/index.svelte

Log for svelte files

You disallowed views to have any kind of stdout or stderr write https://github.com/livebud/bud/commit/2daa288ec5b802418ff200b9301d87c8385e9163

https://github.com/livebud/bud/blob/2419421d5c3326f4eed9ed39e23e7ffa38657978/runtime/generator/view/view_test.go#L116-L117

matthewmueller commented 2 years ago

1. Improve the boot logging

I am thinking about this

I like where this is going! I'm trying to think about what value showing the filename that changed adds to the developer... Honestly I don't think showing it adds much value, you know what file you just changed. Also the file paths could get long.

I'm leaning towards omitting the filename, but keeping the logic you described above, so:

| Ready on http://127.0.0.1:3004 in 106ms (x2)

Where,

On a normal, single file change, the counter Reset if failed to reload or user switched file (eg hello/controller.go to world/controller.go) Increase on success reloads

Log for svelte files

You disallowed views to have any kind of stdout or stderr write https://github.com/livebud/bud/commit/2daa288ec5b802418ff200b9301d87c8385e9163

Oh sorry, this probably was a mistake. Feel free to fix that and edit that test.

Thanks @012e!!

matthewmueller commented 2 years ago

Just tested this. This is seriously amazing Huy. As a user just playing around with this PR, I haven't seen a better reload experience.

There's a few minor tweaks I'd like to make to the code, but I'm going to merge this as is since I want to merge #133 today.

Thank you!!

livebud / bud