Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add therubyracer/therubyrhino to Gemfile when there isn't a JS Runtime available in the system #3619

Closed
wants to merge 1 commit into from

9 participants

@guilleiguaran

This will make the apps usable by default for all Linux users that don't have Node.js installed in the system. (There are a lot of users). See #2963 for details.

Additionally if JRuby is detected therubyrhino is added by default

@wycats
Collaborator

Since the Gemfile is created during the development environment only, and development environments aren't always the same as production environments, I think this should go in the development group.

Thoughts?

@jonleighton
Collaborator

GNOME 3 ships with SpiderMonkey, so it would be good to detect that (which js) too.

@josevalim
Owner

This is already getting too complicated. I am more in favor of execjs raising a meaningful error message (if it doesn't yet).

@jonleighton
Collaborator

Yeah, I agree with that.

@guilleiguaran

The error message in execjs looks fine for me but looks like some new users don't found it very friendly since they are following the Getting Started Guide and they expect having working apps by default.

Actually the purpose of running bundle in new generated apps is have working apps immediately, and in this case the users will have to edit their Gemfile and run bundle a second time just to see the "Welcome aboard" page.

@guilleiguaran

About JRuby, I think it's a different case, therubyrhino is the best default for JRuby in 100% of cases and the JRuby team was planning already send a patch about this, I will create a separated Pull Request for it

@vijaydev
Collaborator

I would suggest we pick one option (therubyracer ?) and put that in the Gemfile without trying to guess the right choice for the user thereby avoiding complications. Let the user choose something else if need be, and we can have a comment indicating so.

@wsouto

I'm more on @josevalim's suggestion. A nice and meaningful message from ExecJS can be suffice for anyone to decide in which way they want to handle the situation: by putting the gem on Gemfile or installing node. But I see the @guilleiguaran's point of having a working app in no time. I saw myself in that situation a few times on Linux. So, can I suggest something like this in case you go this way:

if RUBY_PLATFORM.include? "linux"
  # put therubyracer/therubyrhino here
end

So it will not cause the gem to be installed in a Mac when an app has started on a Linux machine without any JS runtime, which could be a quite common scenario.

A message from ExecJS + the @vijaydev's suggestion of a comment in Gemfile can be nice too. I like this way.

@jonleighton
Collaborator

The thing is, therubyracer takes forever to install. If I am on ubuntu, I'd much rather try to start my app, have that fail, type "sudo apt-get install nodejs", and then succeed, than wait like 10-20 minutes for therubyracer to compile.

@guilleiguaran

@jonleighton agree, therubyracer compiling time was improved with 0.9.x release but still taking minutes to install. On my machine (Macbook Air) it takes 4 minutes to install.

Maybe we can improve the error message in execjs giving posible hints directly in the error message (like add therubyracer to the Gemfile or install Node.js)

@jonleighton
Collaborator

Improving the execjs errors sounds good to me. Closing this here then.

@danielvlopes

Sorry, but improve the error message will not help. In last two months, I had more than a hundred of new students for Rails 3.1 and I answered, at least, 10 times what is a javascript runtime and why they need install it.

For us it's natural but for somebody that probably never opened the terminal it's not (and execjs's message is already good enough when you know what execjs is).

@jonleighton
Collaborator

Perhaps you guys can start a discussion on the Rails core mailing list if you still want to pursue this? There are clearly some different views so would be good to get more discussion of it.

@danielvlopes

I just sent a msg in Rails core but we need to wait for the moderation.

@josevalim
Owner

I am fine with a solution along those lines of the logic is not implemented by Rails. If we could ask exec.js if there is a runtime available and if not, fallback to therubyracer, that would be fine for me. If not, totally -1 for rails checking node, spider monkey versions and etc.

@guilleiguaran

@josevalim We can ask to ExecJS if there is a engine available but we will need make to execjs gem a dependency for railties

@josevalim
Owner
@jrochkind

I think it's really unfortunate that currently on Linux systems rails new gets you an app that results in an error message, rather than a working app. Pre-asset-pipeline, rails new (or 2.x rails app_name) always got you an app that would boot (thanks to sqlite3, among other things).

It's really unfortunate that this is no longer true, and is definitely a poor and discouraging experience for newbies -- the error message does not keep it from being so.

I'd agree that the 'right' way to handle this would be to make 'execjs' a dependency of railties, and then have the generator use execjs's own logic to determine if a ruby runtime is avail, rather than duplicating execjs logic in Rails generator. (ExecJS::Runtimes.autodetect and rescue RuntimeUnavailable).

execjs is a pretty small and pure ruby gem, it doesn't seem that bad to add it as a dependency.

The 'expert' user can install node.js on their system, and then execjs will detect it and therubyracer will never be added to their Gemfile. But for non-expert/newbie users, things should Just Work, even at the cost of a longer 'bundle install' to build therubyracer.

@brentsowers1

Pull Request 4407 (#4407): This is the same change as 4405, just in the 3.1 branch. tenderlove responded to it and I replied. I totally agree with @jrochkind, it's unfortunate that rails new will not generate a runnable app. A total newbie's first impression with Rails shouldn't be frustration that it didn't work out of the box when getting up and running quickly has always been a great feature of Rails, especially when the problem can be easily solved and has already been written. I'm sure once a new user figures out how to add therubyracer to Gemfile, they'll think the same thing I did when this error happened to me, "Why didn't Rails do this? It added all of those other gems to the Gemfile".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 12, 2011
  1. @guilleiguaran

    Add therubyracer/therubyrhino to Gemfile when there isn't a JS Runtim…

    guilleiguaran authored
    …e available in the system. Closes #2963
This page is out of date. Refresh to see the latest.
View
30 railties/lib/rails/generators/app_base.rb
@@ -193,17 +193,47 @@ def ruby_debugger_gemfile_entry
end
def assets_gemfile_entry
+ gem_for_js_runtime = true unless js_runtime_available?
+
<<-GEMFILE.strip_heredoc
# Gems used only for assets and not required
# in production environments by default.
group :assets do
gem 'sass-rails', :git => 'git://github.com/rails/sass-rails.git'
gem 'coffee-rails', :git => 'git://github.com/rails/coffee-rails.git'
+
gem 'uglifier', '>= 1.0.3'
+
+ #{"# JavaScript runtime for CoffeeScript assets and Uglifier" if gem_for_js_runtime}
+ #{"# See https://github.com/sstephenson/execjs for alternative runtimes." if gem_for_js_runtime}
+ #{javascript_runtime_gemfile_entry if gem_for_js_runtime}
end
GEMFILE
end
+ def javascript_runtime_gemfile_entry
+ if defined?(JRUBY_VERSION)
+ "gem 'therubyrhino'"
+ else
+ "gem 'therubyracer'"
+ end
+ end
+
+ def js_runtime_available?
+ # Windows and OS X includes JavaScript Runtime by default
+ return true if RUBY_PLATFORM =~ /mswin32/ || RUBY_PLATFORM =~ /darwin/
+
+ # Prefer therubyrhino when running under JRuby
+ return false if defined?(JRUBY_VERSION)
+
+ # Node.js can be used if is installed in PATH
+ `which node`
+ return true if $?.success?
+
+ # Probably there isn't a javascript installed in the system
+ return false
+ end
+
def javascript_gemfile_entry
"gem '#{options[:javascript]}-rails'" unless options[:skip_javascript]
end
View
11 railties/test/generators/app_generator_test.rb
@@ -189,6 +189,17 @@ def test_config_jdbc_database_when_no_option_given
end
end
+ def test_javascript_runtime_is_added_to_gemfile
+ win_or_osx = (RUBY_PLATFORM =~ /mswin32/ || RUBY_PLATFORM =~ /darwin/)
+ `which node`
+ node = $?.success?
+
+ if defined?(JRUBY_VERSION) || (!win_or_osx && !node)
+ run_generator([destination_root])
+ assert_file "Gemfile", /gem\s+["']theruby(racer|rhino)["']/
+ end
+ end
+
def test_generator_if_skip_active_record_is_given
run_generator [destination_root, "--skip-active-record"]
assert_no_file "config/database.yml"
Something went wrong with that request. Please try again.