Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Add --clean option to vendor:publish command #49329

Conversation

gdebrauwer
Copy link
Contributor

Fixes #48964.

I have a package that uses Vite to build assets. When the package is installed in a project, you run the vendor:publish command to publish those assets. You might end up with the following assets in your public folder:
Screen Shot 2023-12-11 at 16 27 27

When the package's assets change, you have to run the vendor:publish command again to get the new assets in your project. When you do that, you get the following result:
Screen Shot 2023-12-11 at 16 27 51
Because the asset filenames change, the existing old files are not deleted and the new files are just added to the existing directory. This is a problem, as the old files will start to accumulate in that folder.

This PR fixes that by introducing a --clean option to the vendor:publish command. When you use that command option, any existing files in a published directory will be deleted. That way, you end up with the correct and new files in your project:
Screen Shot 2023-12-11 at 16 31 30

@timacdonald
Copy link
Member

timacdonald commented Dec 12, 2023

I'm wondering if we need to consider how this ties in with laravel/vite-plugin#251 to give a holistic framework approach - or at least a consistent naming convention.

@timacdonald
Copy link
Member

timacdonald commented Dec 12, 2023

I love this idea.

My concern is that the implementation will lead to issues during deployments.

Imagine you have a package that has published public/vendor/acme/scripts.abc123.js.

You update the underlying package and it now has a new file it wants to publish, which will be named public/vendor/acme/scripts.xyz789.js.

The following happens...

  1. A client sends a request to your app.
  2. A HTML response containing a reference to scripts.abc123.js is sent back.
  3. You do a deployment which includes an rm -rf public/vendor/acme/ before publishing the new files.
  4. The client's browser finish parsing the response.
  5. The client's browser now tries to load scripts.abc123.js, which 404s.

I would love if we could solve this issue. Making the switch over more atomic is not the solution, as depending on network speeds, it could be a few seconds before the request for the old asset is made.

I could imagine a world where, when publishing assets, we create a laravel.manifest file. This file contains a reference to all the currently published assets.

Then we can offer a vendor:clean / vendor:prune command that has the sole responsibility of removing orphaned files. This command can be run at the end of the deployment process, possibly with a short sleep in your deploy script, to protect against the race condition outlined above.

This is something that laravel/vite-plugin#251 has considered and is built for.

@gdebrauwer
Copy link
Contributor Author

Then we can offer a vendor:clean / vendor:prune command that has the sole responsibility of removing orphaned files. This command can be run at the end of the deployment process, possibly with a short sleep in your deploy script, to protect against the race condition outlined above.

But that means you still will have the old files in your git repository and you have to remember to run that command locally after the deployment process so you can remove the old files from the git repo?

(btw, it was awesome meeting you at LaraconAU 😄)

@taylorotwell taylorotwell marked this pull request as draft December 13, 2023 16:56
@timacdonald timacdonald self-assigned this Dec 15, 2023
@timacdonald
Copy link
Member

@gdebrauwer it was awesome getting to hang with you!

I'm think more when you wouldn't have these committed to your git repository and instead publish them as part of your deployment process.

Will circle back to this shortly and see if I can come up with something.

@daniser
Copy link
Contributor

daniser commented Dec 20, 2023

@timacdonald clean-orphaned-assets binary is great, but I think that there should be artisan vite:cleanup (example name) counterpart for deploying prebuilt assets into environment where Node isn't available.

Edit: I should have read the conversation above beforehand)

@timacdonald
Copy link
Member

I agree this is a good command! We could use it in Forge.

Just wanna make sure it supports my concerns outlined. I plan to come back and get this going (unless someone else addresses the issues first)

@driesvints
Copy link
Member

Please send this to 11.x instead, thanks.

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.

Publish of vite files using "vendor:publish" command does not cleanup old vite files
4 participants