hmdne / roda-sprockets

MIT License
5 stars 1 forks source link

What is the `path_prefix` means? this default settings cause 'r.public gzip: true' not work. #7

Closed zw963 closed 3 years ago

zw963 commented 3 years ago

We still use my own project https://github.com/zw963/marketbet_crawler as a example for describe this issue.

Following is my settings:

https://github.com/zw963/marketbet_crawler/blob/test_roda_sprockets/app/app.rb#L16-L21

Like this:

  plugin :public, gzip:true
  plugin :sprockets, public_path: 'public/',
    opal: true,
    js_compressor: Terser.new,
    css_compressor: :sassc,
    debug: ENV['RACK_ENV'] != 'production'

after run RACK_ENV=production rake assets:precompile, we generated js with gzipped version in public like this:

app-1504fdb5c2f6dd3a661906244baf9f0cd767ca93f92712466f7122574e459724.js
app-1504fdb5c2f6dd3a661906244baf9f0cd767ca93f92712466f7122574e459724.js.gz

Because i am not set path_prefix, it use default value '/assets'

So, if i add <%== javascript_tag 'app' %>, it generate js link like this.

http://localhost:3000/assets/app-1504fdb5c2f6dd3a661906244baf9f0cd767ca93f92712466f7122574e459724.js

I don't know why, this cause a issue which roda never serve gzipped assets.

Following is reproduce:

request (without gzip request header)

curl http://localhost:3000/assets/app-1504fdb5c2f6dd3a661906244baf9f0cd767ca93f92712466f7122574e459724.js --silent --write-out "%{size_download}\n" --output /dev/null 1981984

request (with gzip request header)

╰─ $ curl http://localhost:3000/assets/app-1504fdb5c2f6dd3a661906244baf9f0cd767ca93f92712466f7122574e459724.js --silent -H "Accept-Encoding: gzip,deflate" --write-out "%{size_download}\n" --output /dev/null 1981984

Both cases we get same download size, that is, gzipped assets never served.


But, if we are not use '/assets' prefix, like this, it get expected gzipped version assets.

╰─ $ curl http://localhost:3000/app-1504fdb5c2f6dd3a661906244baf9f0cd767ca93f92712466f7122574e459724.js --silent -H "Accept-Encoding: gzip,deflate" --write-out "%{size_download}\n" --output /dev/null 371748

We can verify this from browser response header.

image


The work around.

we must set path_prefix to '' in roda-sprockets config like this, to let sprockets generate link no '/assets' path prefix.

plugin :sprockets, public_path: 'public/',
    opal: true,
    js_compressor: Terser.new,
    path_prefix: '',
    css_compressor: :sassc,
    debug: ENV['RACK_ENV'] != 'production'

Then it works now.

curl http://localhost:3000/app-1504fdb5c2f6dd3a661906244baf9f0cd767ca93f92712466f7122574e459724.js --silent -H "Accept-Encoding: gzip,deflate" --write-out "%{size_download}\n" --output /dev/null 371748

so, maybe this is a path_prefix default value issue? or other? i don't know, please check, thank you.

zw963 commented 3 years ago

I am so curious why my site was so slow when first time access, now, i know the exact reason.

as you can see, original js assets file is 5.33X (1981984.0/371748) size then gzipped version, this especially important for user like me, which live with a limit connection.

i consider the better solution is, still keep /assets prefix path, instead of fix the bug of gzip not work.

Any idea? maybe we can ask for help from @jeremyevans give some advise.

jeremyevans commented 3 years ago

To me this appears to be a configuration issue in the application, not an issue with roda-sprockets. Either use public_path: 'public/assets' (assuming that works) or use public_path: 'public/', path_prefix: '' (as you have).

zw963 commented 3 years ago

Either use public_path: 'public/assets' (assuming that works) or use public_path: 'public/', path_prefix: '' (as you have).

Cool.

public_path: 'public/assets', this solution is working too, because when run RACK_ENV=production rake assets:precompile, it create assets into public/assets, instead of original public.

Or, another solution. public_path: 'public/', path_prefix: ' '

So, i consider this is a give wrong default value issue, because plugins need user must specify pubilc_path, but, path_prefix is not necessary, this will error prone, e.g. like me, set public_path: '/public' only, which cause gzip not work.

i propose two solution:

  1. remove path_prefix default value, and check it must be set as public_path does
  2. use public_path: 'public/assets' + path_prefix: '/assets' as default value, anyway, this is a totally just working default.

so, what do you guys think?

jeremyevans commented 3 years ago

I can say from reading the README that I would make the same assumption as @zw963, so I don't think @zw963 is doing anything wrong. However, I assume option 1 and option 2 would both break backwards compatibility. So I think @hmdne needs to make a choice as to whether to break backwards compatibility by changing the defaults, or whether to keep backwards compatibility and improve the documentation so other users do not run into this issue.

hmdne commented 3 years ago

There are no defaults for this setting, but the documentation is misleading. public_path should be 'public/assets/' and this could even become a default value. I will amend this shortly.

hmdne commented 3 years ago

Anyway to sum up everything, thank you @zw963 for your bug reports and @jeremyevans for your suggestions, I released v1.1.0.rc1 so you can check if nothing broke. I will check things later in my application as well and then release v1.1.0 final. Please reopen the issues if you think I didn't fix a certain problem.

zw963 commented 3 years ago

@hmdne , one more question, i see we set public_path default value to public/assets/(it suffix with a /)

i want to confirm if this is intended ?

Anyway, for ruby, if we want do specify a file path, we will omit the ending / as a rule, right?

e.g. if we want get manifest file, we can use like this

"#{App.sprockets_options[:public_path]}}/.sprockets-manifest*.json"

but, for now, we need use like

"#{App.sprockets_options[:public_path]}}.sprockets-manifest*.json"

without / as delimiter, it is confusing, though, double // in path is working anyway if we specify like previous form.