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

Production env broken under sprockets-rails >= 3 (Rails 5) #170

Closed
thewoolleyman opened this issue Dec 22, 2015 · 19 comments
Closed

Production env broken under sprockets-rails >= 3 (Rails 5) #170

thewoolleyman opened this issue Dec 22, 2015 · 19 comments

Comments

@thewoolleyman
Copy link
Contributor

(Similar to issue as rails/sprockets-rails#237)

I'm running Rails 5 beta, and found this issue.

When running in Rails production env with config.assets.compile = false, the react_component call in a view blows up. The root cause is because the following line blows up due to Rails.application.assets being nil:

https://github.com/shakacode/react_on_rails/blob/009db0d/lib/react_on_rails/server_rendering_pool.rb#L79

This is apparently new behavior in Sprockets 3 (see link to sprockets-rails issue above), which is why the example app (which uses sprockets-rails 2/Rails 4) still works.

Thanks,
-- Chad

@justin808
Copy link
Member

@thewoolleyman Any chance that you'd like to submit a PR? or maybe branch on the https://github.com/shakacode/react-webpack-rails-tutorial/ showing this can work with Rails 5?

@thewoolleyman
Copy link
Contributor Author

@justin808 I can try, if I can figure out what the fix would be. This seems to be a common problem, but I haven't researched what the fix is. I'm assuming somehow https://github.com/shakacode/react_on_rails/blob/009db0d910c8fef27cd1319f30d5f8f0b7127461/app/assets/javascripts/react_on_rails.js has to get compiled in to the bundle - any ideas how to approach that?

@thewoolleyman
Copy link
Contributor Author

@justin808 You can see on the issue rails/sprockets-rails#237 that several other issues reference it because several other libs have this problem. However, I can't see that any of them have fixed it in a way that would apply to react_on_rails (or at all, most of the issues still open as of now).

@justin808
Copy link
Member

@thewoolleyman within a few weeks, I'll be switching the JS assets over to NPM, so everything will be in the webpack bundle. So maybe don't worry about this one for a few weeks, if the only issue is getting react_on_rails.js in the bundle.

@justin808
Copy link
Member

Here's the code @thewoolleyman referenced. Yes, #{::Rails.application.assets['react_on_rails.js']}; will already be in the server_js_file so no worries with that one.

@thewoolleyman How about skipping the server rendering? Any other issues?

      def create_js_context
        server_js_file = ReactOnRails.configuration.server_bundle_js_file
        if server_js_file.present? && File.exist?(server_js_file)
          bundle_js_code = File.read(server_js_file)
          base_js_code = <<-JS
#{console_polyfill}
          #{bundle_js_code};
#{::Rails.application.assets['react_on_rails.js']};
          JS
          ExecJS.compile(base_js_code)
        else
          if server_js_file.present?
            msg = "You specified server rendering JS file: #{server_js_file}, but it cannot be "\
              "read. You may set the server_bundle_js_file in your configuration to be \"\" to "\
              "avoid this warning"
            Rails.logger.warn msg
            puts msg
          end
          ExecJS.compile("")
        end
      end

@thewoolleyman
Copy link
Contributor Author

@justin808 Not sure what you mean by "How about skipping the server rendering? Any other issues?"

But, if you mean that there's no need to append the react_on_rails.js in this case, then is the fix as easy as just not executing that line if config.assets.compile = false?

@justin808
Copy link
Member

If the only problem you had was the line you referenced https://github.com/shakacode/react_on_rails/blob/009db0d910c8fef27cd1319f30d5f8f0b7127461/app/assets/javascripts/react_on_rails.js, then we only have a problem with server rendering.

I'm not sure how the config.assets.compile comes into play.

@thewoolleyman
Copy link
Contributor Author

I guess it is only with server rendering, but that's still a problem.

config.assets.compile matters, because when (and only when) it is set to false in the Rails config, that causes the error, because Rails.application.assets will be nil in the following line: https://github.com/shakacode/react_on_rails/blob/009db0d/lib/react_on_rails/server_rendering_pool.rb#L79

So, I'm proposing that the fix is to detect if config.assets.compile was set to false (whatever the correct way to do that is), and not even execute that line if it was set to false. This should work if, as you said, "#{::Rails.application.assets['react_on_rails.js']}; will already be in the server_js_file", because this line shouldn't be necessary in that case.

Does that make sense, or am I missing something?

@justin808
Copy link
Member

@thewoolleyman That will be true with v2.0 of react_on_rails. It would be nice to fix this on ASAP on the current version. Basically, if we can't call this:

#{::Rails.application.assets['react_on_rails.js']};

What's the alternative in Rails 5 to get the contents of react_on_rails.js? There's probably some path we can code in.

@justin808
Copy link
Member

@thewoolleyman I will try to do a release later today, so if you have a good workaround for this issue, please let me know.

@thewoolleyman
Copy link
Contributor Author

@justin808 Sorry, I don't really understand it enough to provide a fix. My app is still not close to production, and for now I'm just setting config.assets.compile = true in my Rails production.rb env to avoid the error. This obviously isn't a long-term solution for a real prod environment, but good enough for me for now, and I'll become more concerned about this once I start running on a server env.

Since that is a workaround, we can probably just leave this ticket open and see what solutions other people come up with and perhaps get some ideas from that.

-- Chad

@martyphee
Copy link
Contributor

I'm having the same issue with Rails 4.2.4.

Only workaround right now is turning prerender off. The react_on_rails.js has to be accessible in server_rendering_pool.rb. This could probably be added to the asset precompile list and then accessed through a helper. If I get time I'll play around with that.

@justin808
Copy link
Member

I'm going to be working on 2.0, which is the NPM conversion. When that happens, we won't need the JS asset and this issue will go away. In the meantime, if @martyphee has a PR, that would be AWESOME.

@justin808
Copy link
Member

See #172. Once we get confirmation that this works with older sprockets, we're good to go.

@justin808
Copy link
Member

I tried doing it in this PR: shakacode/react-webpack-rails-tutorial#199

And there's more to it...

See shakacode/react-webpack-rails-tutorial#200

@justin808
Copy link
Member

@robwise and I know this is an issue with Sprockets 3. The only tricky thing is that we need to keep this working with with Sprockets 2, at least for a while.

Related: shakacode/react-webpack-rails-tutorial#200

@robwise
Copy link
Contributor

robwise commented Jan 21, 2016

@justin808 we could try wrapping the whole thing in conditionals that attempt to do some sort of inspection of the rails-sprockets version at runtime? That's super hacky though

@justin808
Copy link
Member

@thewoolleyman, @martyphee Is this still an issue? Can we close this?

@justin808
Copy link
Member

Not an issue since we moved to npm for the JS part of React on Rails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants