Added non-digested version for assets (images, fonts, svg)

[alexeuler]

Problem
When client js uses images in render methods, e.g. <img src='...' /> or in css, e.g. background-image: url(...) these assets fails to load. This happens because rails adds digest hashes to filenames when compiling assets. e.g. img1.jpg becomes img1-dbu097452jf2v2.jpg. When compiling its native css rails transforms all urls and links to work on digested versions, i.e. background-image: url(img1.jpg) becomes background-image: url(img1-dbu097452jf2v2.jpg). However this doesn't happen for js and css files compiled by webpack on the client side and assets fail to load

Solution

Create symlinks of non-digested versions to digested versions when rails assets compile

---

This change is

[justin808]

Let's look at this sample: https://github.com/shakacode/react-webpack-rails-tutorial/blob/master/lib/tasks/assets.rake

I like the idea of putting these in standard tasks for the gem. However, we need to consider the upgrade path for existing ReactOnRails apps. For example, somebody's app might already be doing this, so we need the ability to configure if this done.

Next, we should consider these below (from the link above). Maybe we can prefix the tasks we offer with the namespace react_on_rails::.

This is super awesome! Let's do this once. We'll also need to update the docs. That's actually as or more important than the generator.

CC: @jbhatab @robwise @aaronvb

Rake::Task["assets:precompile"]
  .clear_prerequisites
  .enhance(["assets:compile_environment"])

namespace :assets do
  # In this task, set prerequisites for the assets:precompile task
  task compile_environment: :webpack do
    Rake::Task["assets:environment"].invoke
  end

  desc "Compile assets with webpack"
  task :webpack do
    sh "cd client && npm run build:production:client"
    sh "cd client && npm run build:production:server"
    sh "mkdir -p public/assets"

    # Critical to manually copy non js/css assets to public/assets as we don't want to fingerprint them
    sh "cp -rf app/assets/webpack/*.woff* app/assets/webpack/*.svg app/assets/webpack/*.ttf "\
       "app/assets/webpack/*.eot* public/assets"

    # TODO: We should be gzipping these
  end

  task :clobber do
    rm_rf "#{Rails.application.config.root}/app/assets/webpack"
  end
end

---

Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[coveralls]

Coverage increased (+0.02%) to 85.344% when pulling 5c59be4 on alleycat-at-git:feature/asset_pipeline_digests into e035e54 on shakacode:master.

[alexeuler]

@justin808 I refactored it to make a hook inside React-on-rails gem. Now we don't have these problems. Also added docs and changelog

---

Review status: 0 of 5 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[justin808]

Added more comments.

We need to test this as a migration to https://github.com/shakacode/react-webpack-rails-tutorial/

---

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

docs/additional-reading/rails-assets.md, line 19 [r2] (raw file):
Will the default value be nil or otherwise? if this is not specified. Does this require a major version bump?

---

lib/react_on_rails/configuration.rb, line 60 [r1] (raw file):
OK -- default to nil, so this is not done. It's opt-in.

---

lib/tasks/assets.rake, line 2 [r2] (raw file):
shouldn't we be checking if the param is nil, so this is not done?

---

lib/generators/react_on_rails/templates/base/base/lib/tasks/assets.rake.tt, line 27 [r1] (raw file):
looks like we're duplicating this?

---

Comments from Reviewable

[coveralls]

Coverage increased (+0.02%) to 85.344% when pulling 9f667aa on alleycat-at-git:feature/asset_pipeline_digests into e035e54 on shakacode:master.

[alexeuler]

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

docs/additional-reading/rails-assets.md, line 19 [r2] (raw file):
I guess I will default it to images, fonts and svgs. It doesn't hurt anything and totally backwards-compatible.

---

lib/react_on_rails/configuration.rb, line 60 [r1] (raw file):
Will change that to to images, fonts and svgs

---

lib/tasks/assets.rake, line 2 [r2] (raw file):
Yes! Nice comment!

---

lib/generators/react_on_rails/templates/base/base/lib/tasks/assets.rake.tt, line 27 [r1] (raw file):
Can you explain what do we duplicate here?

---

Comments from Reviewable

[justin808]

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

lib/generators/react_on_rails/templates/base/base/lib/tasks/assets.rake.tt, line 27 [r1] (raw file):
this is the generator -- why are we creating this task in the generated app when it seems to be in the gem

---

Comments from Reviewable

[coveralls]

Coverage increased (+0.02%) to 85.344% when pulling af10567 on alleycat-at-git:feature/asset_pipeline_digests into e035e54 on shakacode:master.

[coveralls]

Coverage increased (+0.02%) to 85.344% when pulling af10567 on alleycat-at-git:feature/asset_pipeline_digests into e035e54 on shakacode:master.

[alexeuler]

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion.

---

lib/generators/react_on_rails/templates/base/base/lib/tasks/assets.rake.tt, line 27 [r1] (raw file):
Symlinking task is almost 100% configurable via ReactOnRails.config, this task however cannot be configured that easy, so I'd rather leave it here - this way framework user could easily override it. But if you think it makes sense to move it to the gem, I can do that.

---

Comments from Reviewable

[robwise]

Reviewed 4 of 5 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

lib/react_on_rails/configuration.rb, line 60 [r3] (raw file):
Rails already creates non-digest versions of svgs, eots, woffs, and ttfs, doesn't it? It's just the picture types?

---

lib/tasks/assets.rake, line 11 [r3] (raw file):
@alleycat-at-git will this also pick up the gzipped assets and provide non-fingerprinted versions of that? We need to make sure we make gzipped versions of the non-digested assets as well for the server to use.

---

lib/tasks/assets.rake, line 15 [r3] (raw file):
minor thing, but it's less typing to do Rails.root.join('public/assets', logical_path)

---

lib/tasks/assets.rake, line 17 [r3] (raw file):
we're sure these will get wiped every time we recompile, right?

Like what if:

• we put an image in our client build
• webpack outputs the image to app/assets/webpack
• rails compiles and places digested version of this image in public/assets
• we create a non-digested symlink to this file in public/assets
• we delete the source image in our client build, so webpack no longer outputs it to app/assets/webpack
• rails compiles and does not place any image in the public/assets folder

would the non-digested symlink we made now be invalid and pointing to nowhere? Or will it have gotten deleted?

---

Comments from Reviewable

[alexeuler]

Review status: 4 of 5 files reviewed at latest revision, 5 unresolved discussions.

---

lib/react_on_rails/configuration.rb, line 60 [r3] (raw file):
I guess it digests everything - here's the assets of a brand new rails project
123-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.svg
123-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.svg.gz
123-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.woff
application-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.css
application-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.css.gz
application-f51a52e2dc61278f78b2b181286b91096cf236e0942e6f57f5c5b8dc24fa28f7.js
application-f51a52e2dc61278f78b2b181286b91096cf236e0942e6f57f5c5b8dc24fa28f7.js.gz

Anyway the comment was useful, thanks! I updated the method to be more cautious and overwrite only symlinks

---

lib/tasks/assets.rake, line 11 [r3] (raw file):
Oh yes, definitely - the way it is used we also symlink all gz versions as we don't specify the end of line in the regex

---

lib/tasks/assets.rake, line 15 [r3] (raw file):
Done.

---

lib/tasks/assets.rake, line 17 [r3] (raw file):
In this case the image despite being removed from webpack will still reside in public/assets. What we can do though is check for broken symlinks each time we recompile the assets.

---

Comments from Reviewable

[alexeuler]

Review status: 4 of 5 files reviewed at latest revision, 5 unresolved discussions.

---

lib/tasks/assets.rake, line 11 [r3] (raw file):
Wrong comment - actually I added the code for gz versions as well

---

Comments from Reviewable

[coveralls]

Coverage increased (+0.02%) to 85.344% when pulling c652fba on alleycat-at-git:feature/asset_pipeline_digests into e035e54 on shakacode:master.

[alexeuler]

@justin808 I checked this PR with react-webpack-rails-tutorial and it totally works. Though we should correct it a bit to remove clutter after we merge this PR

---

Review status: 4 of 5 files reviewed at latest revision, 5 unresolved discussions.

---

Comments from Reviewable

[robwise]

@alleycat-at-git make sure you check with an image that is large enough that webpack won't just inline it, that's how we never caught this before

good job

---

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

lib/react_on_rails/configuration.rb, line 60 [r3] (raw file):
oh okay

---

Comments from Reviewable

[alexeuler]

@robwise Thanks! )

---

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[coveralls]

Coverage increased (+0.02%) to 85.344% when pulling fc23f2e on alleycat-at-git:feature/asset_pipeline_digests into 35e1861 on shakacode:master.

[justin808]

I had a suggestion and question.

Looks AMAZING!

---

Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

docs/additional-reading/rails-assets.md, line 19 [r5] (raw file):
Are symlinks definitely better than just copying the assets on deploy to the given names?

---

lib/tasks/assets.rake, line 22 [r5] (raw file):

if ReactOnRails.configuration.symlink_non_digested_assets_regex.present?

otherwise, if this is set to a "", then that's truthy

---

lib/generators/react_on_rails/templates/base/base/lib/tasks/assets.rake.tt, line 27 [r1] (raw file):
Yes, let's depend on the gem. If somebody needs to change it, they can find the source and copy it.

Otherwise, this is just extra stuff that @jbhatab will have to explain when he creates a video of how the generators work.

---

Comments from Reviewable

[alexeuler]

Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

docs/additional-reading/rails-assets.md, line 19 [r5] (raw file):
I think yes - this just saves some space and ensures that we have absolutely identical assets.

---

lib/tasks/assets.rake, line 22 [r5] (raw file):
This is not a string, but a regex, so if you make it // then it will actually match any asset

---

lib/generators/react_on_rails/templates/base/base/lib/tasks/assets.rake.tt, line 27 [r1] (raw file):
Ok

---

Comments from Reviewable

[alexeuler]

Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

lib/generators/react_on_rails/templates/base/base/lib/tasks/assets.rake.tt, line 27 [r1] (raw file):
@justin808 JIC this is not my code - it was there before this PR.

---

Comments from Reviewable

[justin808]

Reviewed 3 of 3 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[coveralls]

Changes Unknown when pulling 2a36ac5 on alleycat-at-git:feature/asset_pipeline_digests into * on shakacode:master*.

[coveralls]

Changes Unknown when pulling 2a36ac5 on alleycat-at-git:feature/asset_pipeline_digests into * on shakacode:master*.