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
Helper for fragment caching server rendered react components #39
Comments
We add the hash of the webpack main.js and the hash of the serializers to the cache checksum def fetch_component(name, args, options, &block)
Rails.cache.fetch [webpack_checksum, name, Digest::SHA1.hexdigest(args.to_s), options], &block
end
def serializer_checksum
return @serializer_checksum if @serializer_checksum.present? && !Rails.env.development?
digest = Digest::SHA1.new
serializer_files.each { |f| digest.file(f) }
@serializer_checksum = digest.hexdigest
end
private
def serializer_files
Dir.glob(Rails.root.join('app/serializers/**/*.rb'))
end
def webpack_checksum
return @webpack_checksum if @webpack_checksum.present? && !Rails.env.development?
@webpack_checksum = Digest::SHA1.file(Webpack.local_asset_path('main.js')).hexdigest
end better ideas welcome |
@andreasklinger Looking at the code, I'm guessing that you include the serializers because your "args" are not the JSON props (Hash or String) being passed to the component. Any chance that you could elaborate more on your inclusion on the serializers and if there were any performance hot spots in computing the digests. |
Hash SpeedInteresting discussion here:
xxHashPossibly xxhash could help?
Background articles
Rails Source Code References
|
We'll add a view helper method for creating a cache key in the near future. This will be a nice addition for this gem. |
@justin808 You are correct that we choose to use the args vs the serializer output because of the slowness of computing the hash (I'm the one who wrote that code, its been a few months though). This was especially obvious when you assume that both the cache output and the pre-render output are cached (based on the same initial serializer input args), in this case you'd want to minimize overhead. However I think it'd be fine to re-evaluate the use of a hash method here - I think we tried sha1sum of the JSON serializer output at one point, but using the original arguments was the easier choice. |
@robwise @alexfedoseev @dylangrafmyre This "issue" is relevant for our upcoming incorporation of fragment caching. We should aim to create a generic helper function, possibly part of our upcoming "React on Rails Pro" premium subscription. |
This will be solved by the hashes of the files from webpacker. The key is that the JS bundles will have to be separated, and the cache call will requiring specifying the dependent bundles. |
What's the status on this? I am investigating memory issue (~leak) in our app coming from gems and have narrowed it to either this or Delayed Jobs Reference: https://github.com/ASoftCo/leaky-gems/blob/master/README.md |
@corsonknowles A helper for caching is definitely doable. We haven't had enough users like you asking for it. I'd be happy to work with you on this if you like. I don't have any comment on the memory leak other than tons of apps are live on React on Rails and nobody is blaming it. |
Thanks! Our memory issue spontaneously resolved during refactoring to move to Rails 5.1.4, but it was almost certainly related to Delayed Jobs and not server rendering. |
what about having the react component helper to support caching (passing |
I think it's a good idea, that way checking the server bundle can be done automatically under the hood if |
Caching is a part of React on Rails Pro. Contact justin@shakacode.com for more info. |
If only the JS code change, then the cache will not be invalidated. Possible solutions to the problem include:
render_component
support caching of components and be able to resolve dependencies. This is relatively more difficult as it involves determining a dependency tree of what went into the component.This is how Rails does it for views.
https://github.com/rails/cache_digests/tree/master/lib/cache_digests
The text was updated successfully, but these errors were encountered: