jack-pallot / dogpatch

A WordPress starter theme including Webpack (via Laravel Mix), Vue, Babel and Tailwind CSS.
https://github.com/jack-pallot/dogpatch
51 stars 6 forks source link

CSS or JS doesn't seem to compile with yarn hot #3

Open morrislaptop opened 5 years ago

jack-pallot commented 5 years ago

I don't use HMR generally so I haven't noticed this. It's a bit more complicated to use with Laravel Mix on a non Laravel project, because Laravel provides a helper for linking directly to asset files using {{ mix('js/bundle.js') }}. Essentially, any changes made are stored on a different port, you should be able to browse to https://localhost:8080/dist/js/bundle.min.js and see changes sync on that url after running yarn hot.

Please could you confirm whether you're able to make template changes in hot mode? If so, I think I know what the issue is and can replicate the same environment, but it might take a while to get a solid solution working.

morrislaptop commented 5 years ago

Yep I realised that changes are served through HTTP only and aren't put on the disk.

I might try and retrofit the logic from https://github.com/laravel/framework/blob/e04a7ffba8b80b934506783a7d0a161dd52eb2ef/src/Illuminate/Foundation/Mix.php#L10 into queue_theme_assets

Would you like a PR for this?

jack-pallot commented 5 years ago

Absolutely, if you want to give it a go then feel free, otherwise I don't mind taking a look. I think it could be simpler than the Laravel version though because we can make use of the default WP fallback if HMR isn't in use. I have done some testing and I think this is how it roughly works:

I'm wondering if the get_file_contents part could also grab the URL out of the hot file, and check to see if a trailing forward slash exists at the end, then serve that URL. The Laravel version seems to hard code the localhost URL which may not work in some cases, I'm not sure. It might be easier just to hard core for the purposes of getting it working for now.

I think this could exist inside the required.php file as a get_mix_asset_path function or something. At some point the next version will have a different directory structure to allow for automatic updates, but we're not there yet. Placing the function here shouldn't cause issues migrating.

I'm going to keep the issue open while this is worked on if that's cool.

jack-pallot commented 5 years ago

I've managed to get this working in the 0.3 dev branch.

If you want to test this out, you should be able to replace / add the following files from the dev branch:

functions/required.php functions/get-mix-asset.php package.json

It's worth mentioning that webpack will now generate an SSL certificate for the hot module URL, so you'll probably need to verify the certificate in your browser first to see any changes.

morrislaptop commented 5 years ago

Thanks!

On Tue, Sep 24, 2019 at 2:21 PM Jack Pallot notifications@github.com wrote:

I've managed to get this working in the 0.3 dev branch https://github.com/jack-pallot/dogpatch/tree/0.3-dev.

If you want to test this out, you should be able to replace / add the following files from the dev branch:

functions/required.php functions/get-mix-asset.php package.json

It's worth mentioning that webpack will now generate an SSL certificate for the hot module URL, so you'll probably need to verify the certificate in your browser first to see any changes.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/jack-pallot/dogpatch/issues/3?email_source=notifications&email_token=AAAQRX26LYZG4PYZLRR4UPDQLIH4LA5CNFSM4IU64ODKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7OJSQY#issuecomment-534550851, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQRXZWAJ5WEPV3OO6J3ALQLIH4LANCNFSM4IU64ODA .