Fix Misc Bugs Introduced in Pack Generator Refactoring #1515
Conversation
| above to use the automated bundle generation feature. The currently installed version is \ | ||
| #{ReactOnRails::WebpackerUtils.semver_to_string(ReactOnRails::WebpackerUtils.shakapacker_version_as_array)}. | ||
| #{ReactOnRails::Utils.semver_to_string(ReactOnRails::WebpackerUtils.shakapacker_version_as_array)}. |
There was a problem hiding this comment.
@Judahmeek Should we move semver_to_string to WebpackerUtils instead?
|
This is not good that our tests are getting passed despite what module to use! |
semver_to_string method2378e03 to
33117d3
Compare
33117d3 to
61b0478
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pulkitkkr)
lib/react_on_rails/webpacker_utils.rb line 124 at r1 (raw file):
Previously, pulkitkkr (Pulkit) wrote…
@Judahmeek Should we move
semver_to_stringtoWebpackerUtilsinstead?
yes
| def self.shackapacker_version_requirement_met?(required_version) | ||
| req_ver = semver_to_string(required_version) | ||
|
|
||
| Gem::Version.new(shakapacker_version) >= Gem::Version.new(req_ver) |
Summary
Refactoring in PR1509 introduced follwing bugs:
The refactor created
semver_to_stringmethod inReactOnRails::Utilsbut they were referenced usingReactOnRails::WebpackerUtilsshackapacker_version_requirement_met?is checking minimum version >= installed version whereas we want to check installed_version > minimum_version. Further, the logic will fail when a required version is 6.5.3 and the installed is 6.6.1.This PR addresses these critical bugs
This change is