search

gobuffalo / buffalo

Rapid Web Development w/ Go
http://gobuffalo.io
MIT License
8.07k stars 573 forks source link

Clean public folder before build #727

Closed knicklabs closed 6 years ago

knicklabs commented 6 years ago

Expected Behavior

The public/assets directory is the output directory for compiled assets and other assets copied from the assets directory.

It is a best practice to clean the output directory before build. This ensures that only the files actually in use by the project are in the directory.

I would expect the following:

  1. The destination directory only contain the files that are in use by the project. This should include only the compiled assets referenced in public/assets/manifest.json and any files that exist in assets that are not part of the webpack bundle, such as image files that have been included because they are referenced directly in Plush template files.
  2. A compiled binary only contain the files that are in use by the project, as described above.

Actual Behavior

The output directory is not cleaned before build. This results in the following:

  1. Files copied from assets, such as images, that have been deleted from the assets directory permanently remain in the output directory.
  2. Previous versions of compiled assets that are no longer referenced in public/assets/manifest.json permanently remain in the output directory.
  3. The entire public/assets directory is included in the built binary, as can be inspecting by ejecting the assets on build.

Here is an example of what my public/assets directory looks like after a couple days of development.

screen shot 2017-10-31 at 10 48 53 pm

And here is an example of what is bundled in the ejected assets when building for production.

screen shot 2017-10-31 at 10 53 06 pm
paganotoni commented 6 years ago

@knicklabs in https://github.com/gobuffalo/buffalo/commit/96baaab5baeff7dc7c5874c4f5447242c98bf259#diff-53399b34c7dfc25ab7dfa0b83d30ce8epack.config.js.tmpl there was added clean-webpack-plugin that will take care of the assets cleanup, as far as I can tell this should be covered with that enhancement.

knicklabs commented 6 years ago

@paganotoni Fantastic! Apologies for not spotting that. I'm brand new to the community. Been using Buffalo for all of a couple days.

The only issue I see with that is that it's only cleaning the compiled JS and CSS. However, you can add other files to assets and they will be copied over to public/assets as configured here.

Forgive me please if I'm missing something, but because public/assets is an output directory, I avoid writing it to it. Seems that it should be best practice to put all assets, e.g. images, favicons, etc. into assets and let webpack copy it to public/assets. In such a case, shouldn't the entire directory be cleaned?

paganotoni commented 6 years ago

@knicklabs that makes sense, we should clean those images and other files to avoid having them into the compiled binary as well as making trash there. thanks for putting together a great description with this issue, sorry that i didn't spot that detail from it.

if you want to work on this, PR's are always welcome, otherwise i can help if needed. Thanks Again!

knicklabs commented 6 years ago

Thanks @paganotoni. I've taken a stab at addressing the issue.

paganotoni commented 6 years ago

Awesome! I did just read the PR, thanks!

paganotoni commented 6 years ago

closed by #728