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

Improve default for Plug.Static usage in endpoint #5960

Closed
rmoorman opened this issue Oct 28, 2024 · 2 comments · Fixed by #5972
Closed

Improve default for Plug.Static usage in endpoint #5960

rmoorman opened this issue Oct 28, 2024 · 2 comments · Fixed by #5972

Comments

@rmoorman
Copy link
Contributor

Environment

  • Elixir version (elixir -v):
Erlang/OTP 27 [erts-15.1.1] [source] [64-bit] [smp:24:24] [ds:24:24:10] [async-threads:1] [jit:ns]

Elixir 1.17.3 (compiled with Erlang/OTP 27)
  • Phoenix version (mix deps):
* phoenix 1.7.14 (Hex package) (mix)
  locked at 1.7.14 (phoenix) c7859bc5
  • Operating system: Linux

Actual behavior

Currently, running mix assets.deploy results in mix phx.digest to be run. This, in turn, generates gzip compressed files along with other digest artifacts. Nothing wrong with that, of course.

Then, within the endpoint, there is a remark suggesting to set gzip: true inside the endpoint for production.

Setting gzip: true for this plug unconditionally, however, can lead to a quite frustrating situation. When there are digest artifacts found within priv/static, the plug does serve the cached files (as configured of course).
But from a development perspective, this might be quite surprising as the digested app js and css now block the fresh files generated during development.

It just happened to me and it was a little time-sink and a bit hard to track down (everything worked fine in a small app where gzip was set to true a while ago; then, after a while, for local asset bundling debugging purposes I ran the asset scripts, which left me a bit confused as to why my CSS changes weren't be picked up).

Expected behavior

I would suggest having the generated endpoint contain something like

  # Serve at "/" the static files from "priv/static" directory.
  plug Plug.Static,
    at: "/",
    from: :<%= @web_app_name %>,
    gzip: not code_reloading?,
    only: <%= @web_namespace %>.static_paths()

so that the default generated endpoint behaves in a more unsurprising manner.

With this in place, during development (using the code reloading mechanism) the gzipped files are not touched, while in production, the gzipped versions are used.

Please let me know if this is reasonable or something else entirely is to be done instead. I can also submit a PR as well, of course.

Best regards,
Rico Moorman

@josevalim
Copy link
Member

This sounds good to me! PR please?

rmoorman added a commit to rmoorman/phoenix that referenced this issue Nov 9, 2024
Adjust the default for the gzip flag in the generated endpoint to take
code reloading into account.

Fixes phoenixframework#5960
rmoorman added a commit to rmoorman/phoenix that referenced this issue Nov 9, 2024
Adjust the default for the gzip flag in the generated endpoint to take
code reloading into account.

Fixes phoenixframework#5960
@rmoorman
Copy link
Contributor Author

rmoorman commented Nov 9, 2024

Good to go AFAICT

SteffenDE pushed a commit that referenced this issue Nov 26, 2024
Adjust the default for the gzip flag in the generated endpoint to take
code reloading into account.

Fixes #5960
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants