Skip "/packs" when serving static assets

[ivopt]

When using webpacker, assets are under /packs but the app.config.assets.prefix still points to /assets (because it still exists anyway).

With this we'd stop profiling whatever we'd be serving under /packs when serving static assets.

@nateberkopec ☝️

[codecov-io]

Codecov Report

Merging #436 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #436      +/-   ##
==========================================
- Coverage   88.53%   88.48%   -0.06%     
==========================================
  Files          22       22              
  Lines        1335     1337       +2     
==========================================
+ Hits         1182     1183       +1     
- Misses        153      154       +1     

Impacted Files	Coverage Δ
lib/patches/net_patches.rb	62.50% <0.00%> (-4.17%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cb877b...06da75c. Read the comment docs.

[ivopt]

This is a rather silly solution for the problem..
A better solution here would be to find out if the app is using webpacker or not and add this if yes.

Probably, and hopefully, there is something within webpacker that would give me that "/packs" path without me needing to hardcode here...

HELP!

[nateberkopec]

Would if defined?(Webpacker) work?

[ivopt]

Would help indeed.

A simple solution with that could be:

c.skip_paths << "/packs" if defined?(Webpacker)

Problem is: You can tell Webpacker to put those anywhere else, like this:

Webpacker allows me to get that path with:

Webpacker.config.public_output_path # => #<Pathname:/app/public/packs/like/this>

So, I am thinking this needs to be a bit smarter and this is where I start to find my solution ugly:

if defined?(Webpacker)
  public_packs_folder = 
    Webpacker.config.public_output_path.to_s.gsub!("#{app.config.root}/public", "")
  c.skip_paths << public_packs_folder
end

Is there a cleaner way to do that "substring"?

---

EDIT:

if defined?(Webpacker)
  public_packs_folder = 
    Webpacker.config.public_output_path.to_s.gsub!(Webpacker.config.public_path.to_s, "")
  c.skip_paths << public_packs_folder
end

[SamSaffron]

I totally support this, but there is an element of fragility, can we add webpacker to the test suite and have a test to confirm this indeed continues to work?

[nateberkopec]

Probably would just need to add/require Webpacker and set those methods correctly.

[ivopt]

I'm a bit unsure how to add Webpacker to this..

I can't see any other tests being made to the railtie code itself, am I missing something?

[OsamaSayegh]

@LynxEyes testing a Railtie/initializer directly is very tricky if not impossible, but what you can do is extract your code into a small method, put it in railtie_methods.rb and simply call the method in our Railtie. Then in railtie_methods_spec.rb you can call your method as many times as you want and do your tests. You can see b15fe68 for an example.

Adding Webpacker should be as simple as adding it to the gemspec file as a development dependency, but there may be some hidden complexities 🐉 😄

If you want, I can pick this up and try to bring it over the finish line.

[ivopt]

@OsamaSayegh if you have a solid idea of how to finish this, then yes, I fully accept the help! Otherwise I'll follow your tip and try to improve this.

[OsamaSayegh]

Lol, I updated your branch to latest and github closed the PR 😆 I'm working on it.

[OsamaSayegh]

Actually it looks like I pushed my local master to your fork's master, which essentially undid your commits and github closed the PR because your fork became identical and I don't think I have push permissions anymore because the PR is closed 🤦‍♂️ Silly me, sorry about that!

Anyway, I created a new PR #447 with your changes and a test case on top. Will leave to @SamSaffron to do the final review. Thank you for your work here @LynxEyes and sorry for my mess.

[ivopt]

Lolol. Let that who hasn't messed up a few git merges cast the first stone.
Cool, and thank you for adding the test, I'll take it as reference for future commits I may end up making 🤗