Skip to content

Conversation

mike-healy
Copy link
Contributor

This info is based on discoveries using Vite with an app running in Laravel Sail. I needed to call npm run dev from my host machine in order for HMR to work. Others mentioned similar issues with Homestead on the issue discussion here.

@timacdonald
Copy link
Member

I would rather we didn't merge this as it currently is, as I can happily run Vite in Sail without any extra work.

composer create-project laravel/laravel sail-test
cd sail-test
php artisan sail:install
# Put the Vite directive in the `welcome.blade.php` file.
./vendor/bin/sail up
sail npm install && sail npm run dev

The above shows me my site with Vite served assets via HMR.

I think the fix is just a config option that we should be releasing this week, so I'd rather we get that confirmed as the fix and document that rather than pushing everyone to install node on their local machine.

@timacdonald timacdonald requested a review from jessarcher July 4, 2022 00:33
@mike-healy
Copy link
Contributor Author

Totally agree it would be preferable to not require node on the host.
I'll try the combo of that server block in the config, and using sail install and sail run dev later to see if that works for me.

I'm not sure if I'm alone in not realising I need to run the npm commands through Sail. I see now the docs mention it once under the Installing Node section.

Do you think that could be more prominent, or explicitly say that all the node/npm commands should be prefixed with sail?

@mike-healy
Copy link
Contributor Author

@timacdonald did you do that with WSL2? (or Sail on Mac?)
I tried those steps from with the WSL2 Ubuntu VM and it didn't work for me.
My vite.config.js is

import { defineConfig } from 'vite';
import laravel from 'laravel-vite-plugin';

export default defineConfig({
    plugins: [
        laravel([
            'resources/css/app.css',
            'resources/js/app.js',
        ]),
    ],
    server: {
        hmr: {
            host: 'localhost',
        },
    }
});

I am starting the server with ./vendor/bin/sail npm run dev (it starts successfully and shows the output).

In browser the app is attempting to load the HMR server and the assets from http://0.0.0.0:5173 which does not respond.
If I manually change the links to http://localhost:5173 they work. I have also tried without the server block in the Vite config.

@taylorotwell taylorotwell marked this pull request as draft July 5, 2022 02:54
@timacdonald
Copy link
Member

@mike-healy I'm on Mac. WSL2 is a known issue that we are working on improving, which is the intention of this PR: laravel/vite-plugin#42 , which is yet to be tagged.

@mike-healy
Copy link
Contributor Author

Thanks @timacdonald. I'll try it again after that release

@timacdonald
Copy link
Member

Nice. Should be available tomorrow to play with.

@driesvints
Copy link
Member

@mike-healy were you able to try this?

@mike-healy
Copy link
Contributor Author

@driesvints – yes I did. No luck for me unfortunately.
laravel/vite-plugin#49 (comment)

@DarkGhostHunter
Copy link
Contributor

Just a note, WSL2 doesn't push file changes to the host and viceversa, so any file-change notification (inotify) is broken. That includes HMR. The protocol doesn't support it, so there is no fix, thus the use of WSL2 should be avoided at all costs.

Dunno on the macOS side of things.

@timacdonald
Copy link
Member

It shouldn't be avoided at all costs. As per the Vite docs...

server.watch

When running Vite on Windows Subsystem for Linux (WSL) 2, if the project folder resides in a Windows filesystem, you'll need to set this option to { usePolling: true }. This is due to a WSL2 limitation with the Windows filesystem.

@driesvints
Copy link
Member

Any movement here? Otherwise it's best that we close this for now.

@driesvints
Copy link
Member

Gonna close this for now. Feel free to reopen when you want to continue work on this.

@driesvints driesvints closed this Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants