Skip to content

Fix webpacker:check_node to display version error #798

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

Merged
merged 1 commit into from
Sep 19, 2017

Conversation

carlosdp
Copy link
Contributor

Currently, if you have an older version of node installed that is referenced by nodejs instead of node, this code receives an Errno::ENOENT exception from the node -v command, bypassing the node_version.blank? check and telling the user Node is not installed. Instead, we should catch that exception and check for nodejs -v and let them know they need to upgrade versions.

Currently, if you have an older version of node installed that is referenced by `nodejs` instead of `node`, this code receives an Errno::ENOENT exception from the `node -v` command, bypassing the `node_version.blank?` check and telling the user Node is not installed. Instead, we should catch that exception and check for `nodejs -v` and let them know they need to upgrade versions.
Copy link
Contributor

@curvo curvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me. There are number of other issues with this rake task on older systems though but I think they should be addressed in a different PR.

@ytbryan
Copy link
Contributor

ytbryan commented Sep 19, 2017

cc @gauravtiwari, should merge this?

@gauravtiwari gauravtiwari merged commit f3c012a into rails:master Sep 19, 2017
@gauravtiwari
Copy link
Member

Thanks @carlosdp for the PR and @curvo @ytbryan for reviewing 🍰

brasic added a commit to brasic/webpacker that referenced this pull request Dec 14, 2017
Running this task when rails is loaded doesn't behave as expected
because ActiveSupport monkeypatches Kernel#` as follows:
https://github.com/rails/rails/blob/7d75599c875b081f63fc292e454b25e8e42eb60e/activesupport/lib/active_support/core_ext/kernel/agnostics.rb
to suppress raising Errno::ENOENT.  With this monkeypatch in place the
code never reaches the fallback branch for legacy node versions
introduced in rails#798

We can fix this by manually raising the exception that the activesupport
monkeypatch has suppressed.
gauravtiwari pushed a commit that referenced this pull request Dec 15, 2017
* Raise ENOENT like vanilla ruby

Running this task when rails is loaded doesn't behave as expected
because ActiveSupport monkeypatches Kernel#` as follows:
https://github.com/rails/rails/blob/7d75599c875b081f63fc292e454b25e8e42eb60e/activesupport/lib/active_support/core_ext/kernel/agnostics.rb
to suppress raising Errno::ENOENT.  With this monkeypatch in place the
code never reaches the fallback branch for legacy node versions
introduced in #798

We can fix this by manually raising the exception that the activesupport
monkeypatch has suppressed.

* Simplify nested rescues

Using shell syntax to choose either node or nodejs has the extra benefit
of never raising Errno::ENOENT whether or not Kernel#` has been
monkeypatched by ActiveSupport.
KingTiger001 added a commit to KingTiger001/Rails-web-pack-project that referenced this pull request Jan 15, 2023
* Raise ENOENT like vanilla ruby

Running this task when rails is loaded doesn't behave as expected
because ActiveSupport monkeypatches Kernel#` as follows:
https://github.com/rails/rails/blob/7d75599c875b081f63fc292e454b25e8e42eb60e/activesupport/lib/active_support/core_ext/kernel/agnostics.rb
to suppress raising Errno::ENOENT.  With this monkeypatch in place the
code never reaches the fallback branch for legacy node versions
introduced in rails/webpacker#798

We can fix this by manually raising the exception that the activesupport
monkeypatch has suppressed.

* Simplify nested rescues

Using shell syntax to choose either node or nodejs has the extra benefit
of never raising Errno::ENOENT whether or not Kernel#` has been
monkeypatched by ActiveSupport.
smartech7 pushed a commit to smartech7/ruby-webpacker that referenced this pull request Aug 4, 2023
* Raise ENOENT like vanilla ruby

Running this task when rails is loaded doesn't behave as expected
because ActiveSupport monkeypatches Kernel#` as follows:
https://github.com/rails/rails/blob/7d75599c875b081f63fc292e454b25e8e42eb60e/activesupport/lib/active_support/core_ext/kernel/agnostics.rb
to suppress raising Errno::ENOENT.  With this monkeypatch in place the
code never reaches the fallback branch for legacy node versions
introduced in rails/webpacker#798

We can fix this by manually raising the exception that the activesupport
monkeypatch has suppressed.

* Simplify nested rescues

Using shell syntax to choose either node or nodejs has the extra benefit
of never raising Errno::ENOENT whether or not Kernel#` has been
monkeypatched by ActiveSupport.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants