Skip to content

Commit 45821a2

Browse files
justin808claude
andauthored
Add lockfile version resolution for exact version checking (#1898)
* Add lockfile version resolution for exact version checking Similar to shakacode/shakapacker#170, this adds support for resolving exact package versions from lockfiles (yarn.lock and package-lock.json) when checking version compatibility between the gem and npm package. Key improvements: - Adds lockfile parsing to NodePackageVersion class - Resolves exact versions from yarn.lock (v1 format) - Resolves exact versions from package-lock.json (v1, v2, v3 formats) - Falls back to package.json version if lockfiles are unavailable - Prefers yarn.lock over package-lock.json when both exist - Supports both react-on-rails and react-on-rails-pro packages This enhancement improves version constraint checking by using the exact resolved version from lockfiles instead of semver ranges in package.json, making version mismatch detection more accurate. Test coverage includes: - Yarn.lock v1 parsing - Package-lock.json v1 and v2 format parsing - Pro package version resolution - Lockfile preference order - Fallback to package.json when no lockfile exists 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Improve lockfile parsing robustness and error handling - Fix error handling in version_from_package_lock to safely check for version key existence using safe navigation operator - Improve lockfile path construction using File.dirname instead of ".." for more robust path resolution - Add ClassLength RuboCop disable for NodePackageVersion class These changes address code review feedback to make the implementation more robust and handle edge cases better. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Fix lockfile path resolution and add comprehensive edge case tests Path Resolution Fixes: - Fix lockfile path construction to look in same directory as package.json - Use base_dir from node_modules_location instead of File.dirname - Prevent resolving to filesystem root when node_modules_location is empty - Ensure lockfiles are found next to package.json as expected Package-lock.json v1 Fix: - Fix dependency_data type checking (can be Hash or String in v1) - Use is_a?(Hash) check before calling key? method New Test Coverage: - Similar package names (react-on-rails vs react-on-rails-pro) - Package-lock.json v1 format parsing - Malformed yarn.lock handling (graceful fallback) - Malformed package-lock.json handling (graceful fallback) The regex pattern ^"?package-name@ already ensures exact matching because @ is the delimiter, preventing "react-on-rails" from matching "react-on-rails-pro". Added test to verify this behavior. All 65 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Fix lockfile resolution for local paths to prevent CI failures Problem: - CI was failing because yarn.lock contains version "0.0.0" for local links - The resolve_version method was checking lockfiles before detecting local paths - This caused "0.0.0" to be returned instead of the local path from package.json Solution: - Check if package.json version is a local path/URL BEFORE resolving from lockfiles - Add local_path_or_url_version? helper method - Skip lockfile resolution for local paths since they have placeholder versions This fixes the CI failures where spec/dummy uses "link:.yalc/react-on-rails" and yarn.lock contains version "0.0.0" for this local link. All 65 tests still passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
1 parent 1a84b4a commit 45821a2

14 files changed

+456
-5
lines changed

lib/react_on_rails/version_checker.rb

Lines changed: 128 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,19 +212,36 @@ def package_json_location
212212
"Package.json location: #{VersionChecker::NodePackageVersion.package_json_path}"
213213
end
214214

215+
# rubocop:disable Metrics/ClassLength
215216
class NodePackageVersion
216-
attr_reader :package_json
217+
attr_reader :package_json, :yarn_lock, :package_lock
217218

218219
def self.build
219-
new(package_json_path)
220+
new(package_json_path, yarn_lock_path, package_lock_path)
220221
end
221222

222223
def self.package_json_path
223224
Rails.root.join(ReactOnRails.configuration.node_modules_location, "package.json")
224225
end
225226

226-
def initialize(package_json)
227+
def self.yarn_lock_path
228+
# Lockfiles are in the same directory as package.json
229+
# If node_modules_location is empty, use Rails.root
230+
base_dir = ReactOnRails.configuration.node_modules_location.presence || ""
231+
Rails.root.join(base_dir, "yarn.lock").to_s
232+
end
233+
234+
def self.package_lock_path
235+
# Lockfiles are in the same directory as package.json
236+
# If node_modules_location is empty, use Rails.root
237+
base_dir = ReactOnRails.configuration.node_modules_location.presence || ""
238+
Rails.root.join(base_dir, "package-lock.json").to_s
239+
end
240+
241+
def initialize(package_json, yarn_lock = nil, package_lock = nil)
227242
@package_json = package_json
243+
@yarn_lock = yarn_lock
244+
@package_lock = package_lock
228245
end
229246

230247
def raw
@@ -238,10 +255,16 @@ def raw
238255
deps = parsed["dependencies"]
239256

240257
# Check for react-on-rails-pro first (Pro takes precedence)
241-
return @raw = deps["react-on-rails-pro"] if deps.key?("react-on-rails-pro")
258+
if deps.key?("react-on-rails-pro")
259+
@raw = resolve_version(deps["react-on-rails-pro"], "react-on-rails-pro")
260+
return @raw
261+
end
242262

243263
# Fall back to react-on-rails
244-
return @raw = deps["react-on-rails"] if deps.key?("react-on-rails")
264+
if deps.key?("react-on-rails")
265+
@raw = resolve_version(deps["react-on-rails"], "react-on-rails")
266+
return @raw
267+
end
245268

246269
# Neither package found
247270
msg = "No 'react-on-rails' or 'react-on-rails-pro' entry in the dependencies of " \
@@ -314,6 +337,105 @@ def parts
314337

315338
private
316339

340+
# Resolve version from lockfiles if available, otherwise use package.json version
341+
# rubocop:disable Metrics/CyclomaticComplexity
342+
def resolve_version(package_json_version, package_name)
343+
# If package.json specifies a local path or URL, don't try to resolve from lockfiles
344+
# Lockfiles may contain placeholder versions like "0.0.0" for local links
345+
return package_json_version if local_path_or_url_version?(package_json_version)
346+
347+
# Try yarn.lock first
348+
if yarn_lock && File.exist?(yarn_lock)
349+
lockfile_version = version_from_yarn_lock(package_name)
350+
return lockfile_version if lockfile_version
351+
end
352+
353+
# Try package-lock.json
354+
if package_lock && File.exist?(package_lock)
355+
lockfile_version = version_from_package_lock(package_name)
356+
return lockfile_version if lockfile_version
357+
end
358+
359+
# Fall back to package.json version
360+
package_json_version
361+
end
362+
# rubocop:enable Metrics/CyclomaticComplexity
363+
364+
# Check if a version string represents a local path or URL
365+
def local_path_or_url_version?(version)
366+
return false if version.nil?
367+
368+
version.include?("/") && !version.start_with?("npm:")
369+
end
370+
371+
# Parse version from yarn.lock
372+
# Looks for entries like:
373+
# react-on-rails@^16.1.1:
374+
# version "16.1.1"
375+
# The pattern ensures exact package name match to avoid matching similar names
376+
# (e.g., "react-on-rails" won't match "react-on-rails-pro")
377+
# rubocop:disable Metrics/CyclomaticComplexity
378+
def version_from_yarn_lock(package_name)
379+
return nil unless yarn_lock && File.exist?(yarn_lock)
380+
381+
in_package_block = false
382+
File.foreach(yarn_lock) do |line|
383+
# Check if we're starting the block for our package
384+
# Pattern: optionally quoted package name, followed by @, ensuring it's not followed by more word chars
385+
# This prevents "react-on-rails" from matching "react-on-rails-pro"
386+
if line.match?(/^"?#{Regexp.escape(package_name)}@/)
387+
in_package_block = true
388+
next
389+
end
390+
391+
# If we're in the package block, look for the version line
392+
if in_package_block
393+
# Version line looks like: version "16.1.1"
394+
if (match = line.match(/^\s+version\s+"([^"]+)"/))
395+
return match[1]
396+
end
397+
398+
# If we hit a blank line or new package, we've left the block
399+
break if line.strip.empty? || (line[0] != " " && line[0] != "\t")
400+
end
401+
end
402+
403+
nil
404+
end
405+
# rubocop:enable Metrics/CyclomaticComplexity
406+
407+
# Parse version from package-lock.json
408+
# Supports both v1 (dependencies) and v2/v3 (packages) formats
409+
# rubocop:disable Metrics/CyclomaticComplexity
410+
def version_from_package_lock(package_name)
411+
return nil unless package_lock && File.exist?(package_lock)
412+
413+
begin
414+
parsed = JSON.parse(File.read(package_lock))
415+
416+
# Try v2/v3 format first (packages)
417+
if parsed["packages"]
418+
# Look for node_modules/package-name entry
419+
node_modules_key = "node_modules/#{package_name}"
420+
package_data = parsed["packages"][node_modules_key]
421+
return package_data["version"] if package_data&.key?("version")
422+
end
423+
424+
# Fall back to v1 format (dependencies)
425+
if parsed["dependencies"]
426+
dependency_data = parsed["dependencies"][package_name]
427+
# In v1, the dependency can be a hash with a "version" key
428+
return dependency_data["version"] if dependency_data.is_a?(Hash) && dependency_data.key?("version")
429+
end
430+
rescue JSON::ParserError
431+
# If we can't parse the lockfile, fall back to package.json version
432+
nil
433+
end
434+
435+
nil
436+
end
437+
# rubocop:enable Metrics/CyclomaticComplexity
438+
317439
def package_installed?(package_name)
318440
return false unless File.exist?(package_json)
319441

@@ -348,5 +470,6 @@ def parsed_package_contents
348470
end
349471
end
350472
end
473+
# rubocop:enable Metrics/ClassLength
351474
end
352475
end
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "test-app",
3+
"version": "1.0.0",
4+
"this is not valid JSON because of the trailing comma",
5+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
2+
# yarn lockfile v1
3+
4+
This is not a valid yarn.lock format
5+
Just some random text here

spec/react_on_rails/fixtures/pro_semver_caret_package-lock.json

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
2+
# yarn lockfile v1
3+
4+
5+
react-on-rails-pro@^16.1.1:
6+
version "16.1.1"
7+
resolved "https://registry.yarnpkg.com/react-on-rails-pro/-/react-on-rails-pro-16.1.1.tgz"
8+
integrity sha512-abc123def456ghi789jkl012mno345pqr678stu901vwx234yz=

spec/react_on_rails/fixtures/semver_caret_package-lock.json

Lines changed: 49 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"name": "test-app",
3+
"version": "1.0.0",
4+
"lockfileVersion": 1,
5+
"requires": true,
6+
"dependencies": {
7+
"babel": {
8+
"version": "6.23.0",
9+
"resolved": "https://registry.npmjs.org/babel/-/babel-6.23.0.tgz",
10+
"integrity": "sha1-0NHn2APpdHZb7qMjLU4VPA77kPQ="
11+
},
12+
"react-on-rails": {
13+
"version": "1.2.3",
14+
"resolved": "https://registry.npmjs.org/react-on-rails/-/react-on-rails-1.2.3.tgz",
15+
"integrity": "sha512-abc123def456ghi789jkl012mno345pqr678stu901vwx234yz="
16+
},
17+
"webpack": {
18+
"version": "1.15.0",
19+
"resolved": "https://registry.npmjs.org/webpack/-/webpack-1.15.0.tgz",
20+
"integrity": "sha1-v4SbvGJWkYqkKVBKjNJlJQQNqZg="
21+
}
22+
}
23+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
2+
# yarn lockfile v1
3+
4+
5+
babel@^6.3.26:
6+
version "6.23.0"
7+
resolved "https://registry.yarnpkg.com/babel/-/babel-6.23.0.tgz"
8+
integrity sha1-0NHn2APpdHZb7qMjLU4VPA77kPQ=
9+
10+
react-on-rails@^1.2.3:
11+
version "1.2.3"
12+
resolved "https://registry.yarnpkg.com/react-on-rails/-/react-on-rails-1.2.3.tgz"
13+
integrity sha512-abc123def456ghi789jkl012mno345pqr678stu901vwx234yz=
14+
15+
webpack@^1.12.8:
16+
version "1.15.0"
17+
resolved "https://registry.yarnpkg.com/webpack/-/webpack-1.15.0.tgz"
18+
integrity sha1-v4SbvGJWkYqkKVBKjNJlJQQNqZg=

spec/react_on_rails/fixtures/semver_exact_package-lock.json

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"dependencies": {
3+
"react-on-rails": "16.1.1"
4+
}
5+
}

0 commit comments

Comments
 (0)