Add an npm ci --dry-run in aio app pack #898
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds a compatibility check between package.json and package-lock.json files during the aio app pack command. The validation runs npm ci --dry-run to detect mismatches early, before packaging begins, preventing install failures in downstream processes.
Changes:
- Added
validatePackageLockCompatibility()method that runsnpm ci --dry-runwhenpackage-lock.jsonexists - Integrated validation as the first step in the pack command workflow, before artifact creation
- Added comprehensive test coverage for validation success, failure with various error types, and skip scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/commands/app/pack.js | Added validatePackageLockCompatibility() method and integrated it into the pack workflow to validate package file compatibility before packaging |
| test/commands/app/pack.test.js | Updated all existing tests to mock the new npm ci validation step and added four new test cases covering validation skip, failure with stderr, failure without stderr, and failure without error details |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| this.spinner.start('Validating package.json and package-lock.json compatibility...') | ||
| try { | ||
| await execa('npm', ['ci', '--dry-run'], { cwd: process.cwd() }) |
There was a problem hiding this comment.
This looks good! I guess npm versions should not be an issue since we can assume that they created their package-lock with whatever npm version they are using and its their issue to fix if there is a mismatch.
pru55e11
left a comment
pru55e11
left a comment
There was a problem hiding this comment.
Looks great @riddhi2910!
| command.config = { runHook } | ||
| await command.run() | ||
|
|
||
| expect(execa).toHaveBeenCalledWith('npm', ['ci', '--dry-run'], { cwd: expect.any(String) }) |
There was a problem hiding this comment.
Quick question- if the --no-lock-file case means "do not include lock file in the package", why do we run the npm ci --dry-run ? I wonder if we should skip the compatibility check, if that flag is set?
Or if the intention is to validate even when we are not packaging the lock file, that's totally fine, but may be a good idea to document that in the flag help text?
There was a problem hiding this comment.
That makes sense , with --no-lock-file Lock failures wont happen, and skipping the check respects what the customer wants. I am now thinking it’s better to remove the validation when --no-lock-file is used as well. This was really helpful, thanks!
cc: @purplecabbage @MichaelGoberling
There was a problem hiding this comment.
Yes agree, imo we should skip if we're not including the lock file
Description
This PR adds a compatibility check to the aio app pack command.
It runs npm ci --dry-run to make sure package.json and package-lock.json match before packaging, helping catch issues early and avoid install failures later.
Motivation and Context
https://jira.corp.adobe.com/browse/ACNA-4288
How Has This Been Tested?
Added tests covering validation success, failure, and skip scenarios, with full coverage and all tests passing.
Manually verified that validation passes for compatible files, fails with clear errors for incompatible ones, and is skipped when package-lock.json is missing.
Screenshots (if appropriate):
Types of changes
Checklist: