Skip to content

Commit 5fcbdaf

Browse files
dermatzdermatz
authored
feat: add npm sync validation to NodePackageManager and theme builders (#93)
* feat: add npm sync validation to NodePackageManager and theme builders * fix: correct npm sync validation tests to install packages
1 parent 7e0b57e commit 5fcbdaf

4 files changed

Lines changed: 217 additions & 9 deletions

File tree

‎.github/workflows/magento-compatibility.yml‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,90 @@ jobs:
176176
echo "Test Inspector status command:"
177177
bin/magento mageforge:theme:inspector status
178178
179+
- name: Test npm Sync Validation
180+
working-directory: magento2
181+
run: |
182+
echo "Setting up Node.js for npm testing..."
183+
export NVM_DIR="$HOME/.nvm"
184+
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
185+
186+
# Install Node.js if not available
187+
if ! command -v node &> /dev/null; then
188+
curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.1/install.sh | bash
189+
export NVM_DIR="$HOME/.nvm"
190+
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
191+
nvm install 20
192+
nvm use 20
193+
fi
194+
195+
echo "Node version: $(node --version)"
196+
echo "npm version: $(npm --version)"
197+
198+
echo "Test npm sync validation with Magento/blank theme..."
199+
THEME_PATH="app/design/frontend/Magento/blank"
200+
201+
# Create a simple package.json for testing
202+
cat > ${THEME_PATH}/package.json << 'EOF'
203+
{
204+
"name": "magento-blank-theme",
205+
"version": "1.0.0",
206+
"description": "Test theme for npm sync validation",
207+
"dependencies": {
208+
"jquery": "^3.7.1"
209+
}
210+
}
211+
EOF
212+
213+
echo "Initial npm install to create package-lock.json and node_modules..."
214+
cd ${THEME_PATH}
215+
npm install
216+
217+
echo "Verifying initial sync status (should be in sync)..."
218+
if npm ls --depth=0 > /dev/null 2>&1; then
219+
echo "✓ Initial state: node_modules in sync"
220+
else
221+
echo "✗ Initial state should be in sync"
222+
exit 1
223+
fi
224+
225+
echo "Simulating out-of-sync state by adding new dependency..."
226+
# Add a new dependency to package.json that's not in node_modules
227+
cat > package.json << 'EOF'
228+
{
229+
"name": "magento-blank-theme",
230+
"version": "1.0.0",
231+
"description": "Test theme for npm sync validation",
232+
"dependencies": {
233+
"jquery": "^3.7.1",
234+
"lodash": "^4.17.21"
235+
}
236+
}
237+
EOF
238+
239+
# Update lock file without installing (simulates a git pull with updated lock file)
240+
npm install --package-lock-only
241+
242+
echo "Verifying out-of-sync state (should fail)..."
243+
if npm ls --depth=0 > /dev/null 2>&1; then
244+
echo "✗ Should detect out-of-sync state"
245+
exit 1
246+
else
247+
echo "✓ Correctly detected out-of-sync state (exit code: $?)"
248+
fi
249+
250+
echo "Testing that npm ci fixes the sync issue..."
251+
npm ci
252+
253+
if npm ls --depth=0 > /dev/null 2>&1; then
254+
echo "✓ npm ci successfully synchronized node_modules"
255+
else
256+
echo "✗ npm ci failed to synchronize"
257+
exit 1
258+
fi
259+
260+
echo "✓ npm sync validation tests passed"
261+
cd ../../../../../../
262+
179263
- name: Test Summary
180264
run: |
181265
echo "MageForge module compatibility test with Magento ${{ matrix.magento-version }} completed"
@@ -336,6 +420,90 @@ jobs:
336420
echo "Test Inspector status command:"
337421
bin/magento mageforge:theme:inspector status
338422
423+
- name: Test npm Sync Validation
424+
working-directory: magento2
425+
run: |
426+
echo "Setting up Node.js for npm testing..."
427+
export NVM_DIR="$HOME/.nvm"
428+
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
429+
430+
# Install Node.js if not available
431+
if ! command -v node &> /dev/null; then
432+
curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.1/install.sh | bash
433+
export NVM_DIR="$HOME/.nvm"
434+
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
435+
nvm install 20
436+
nvm use 20
437+
fi
438+
439+
echo "Node version: $(node --version)"
440+
echo "npm version: $(npm --version)"
441+
442+
echo "Test npm sync validation with Magento/blank theme..."
443+
THEME_PATH="app/design/frontend/Magento/blank"
444+
445+
# Create a simple package.json for testing
446+
cat > ${THEME_PATH}/package.json << 'EOF'
447+
{
448+
"name": "magento-blank-theme",
449+
"version": "1.0.0",
450+
"description": "Test theme for npm sync validation",
451+
"dependencies": {
452+
"jquery": "^3.7.1"
453+
}
454+
}
455+
EOF
456+
457+
echo "Initial npm install to create package-lock.json and node_modules..."
458+
cd ${THEME_PATH}
459+
npm install
460+
461+
echo "Verifying initial sync status (should be in sync)..."
462+
if npm ls --depth=0 > /dev/null 2>&1; then
463+
echo "✓ Initial state: node_modules in sync"
464+
else
465+
echo "✗ Initial state should be in sync"
466+
exit 1
467+
fi
468+
469+
echo "Simulating out-of-sync state by adding new dependency..."
470+
# Add a new dependency to package.json that's not in node_modules
471+
cat > package.json << 'EOF'
472+
{
473+
"name": "magento-blank-theme",
474+
"version": "1.0.0",
475+
"description": "Test theme for npm sync validation",
476+
"dependencies": {
477+
"jquery": "^3.7.1",
478+
"lodash": "^4.17.21"
479+
}
480+
}
481+
EOF
482+
483+
# Update lock file without installing (simulates a git pull with updated lock file)
484+
npm install --package-lock-only
485+
486+
echo "Verifying out-of-sync state (should fail)..."
487+
if npm ls --depth=0 > /dev/null 2>&1; then
488+
echo "✗ Should detect out-of-sync state"
489+
exit 1
490+
else
491+
echo "✓ Correctly detected out-of-sync state (exit code: $?)"
492+
fi
493+
494+
echo "Testing that npm ci fixes the sync issue..."
495+
npm ci
496+
497+
if npm ls --depth=0 > /dev/null 2>&1; then
498+
echo "✓ npm ci successfully synchronized node_modules"
499+
else
500+
echo "✗ npm ci failed to synchronize"
501+
exit 1
502+
fi
503+
504+
echo "✓ npm sync validation tests passed"
505+
cd ../../../../../../
506+
339507
- name: Test Summary
340508
run: |
341509
echo "MageForge module compatibility test with Magento 2.4.8 completed"

‎src/Service/NodePackageManager.php‎

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ public function __construct(
2222
* Install node modules in the specified directory
2323
*
2424
* Uses npm ci if package-lock.json exists, otherwise falls back to npm install
25+
*
26+
* @param string $path
27+
* @param SymfonyStyle $io
28+
* @param bool $isVerbose
29+
* @return bool
2530
*/
2631
public function installNodeModules(string $path, SymfonyStyle $io, bool $isVerbose): bool
2732
{
@@ -51,11 +56,46 @@ public function installNodeModules(string $path, SymfonyStyle $io, bool $isVerbo
5156
}
5257
}
5358

59+
/**
60+
* Check if node_modules is in sync with package-lock.json
61+
*
62+
* Verifies that installed packages match the lock file by checking:
63+
* 1. node_modules directory exists
64+
* 2. package-lock.json exists
65+
* 3. All packages are installed with correct versions (via npm ls)
66+
*
67+
* @param string $path
68+
* @return bool
69+
*/
70+
public function isNodeModulesInSync(string $path): bool
71+
{
72+
if (!$this->fileDriver->isDirectory($path . '/node_modules')) {
73+
return false;
74+
}
75+
76+
if (!$this->fileDriver->isExists($path . '/package-lock.json')) {
77+
return true;
78+
}
79+
80+
$currentDir = getcwd();
81+
chdir($path);
82+
83+
try {
84+
$this->shell->execute('npm ls --depth=0 --json > /dev/null 2>&1');
85+
chdir($currentDir);
86+
return true;
87+
} catch (\Exception $e) {
88+
chdir($currentDir);
89+
return false;
90+
}
91+
}
92+
5493
/**
5594
* Check for outdated npm packages and report them
5695
*
57-
* Note: npm outdated returns non-zero exit code when packages are outdated,
58-
* so exceptions are caught and ignored
96+
* @param string $path
97+
* @param SymfonyStyle $io
98+
* @return void
5999
*/
60100
public function checkOutdatedPackages(string $path, SymfonyStyle $io): void
61101
{
@@ -69,7 +109,7 @@ public function checkOutdatedPackages(string $path, SymfonyStyle $io): void
69109
$io->writeln($outdated);
70110
}
71111
} catch (\Exception $e) {
72-
// Ignore errors from npm outdated as it returns non-zero when packages are outdated
112+
// npm outdated returns non-zero exit code when packages are outdated
73113
}
74114

75115
chdir($currentDir);

‎src/Service/ThemeBuilder/HyvaThemes/Builder.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,10 @@ public function autoRepair(string $themePath, SymfonyStyle $io, OutputInterface
155155
{
156156
$tailwindPath = rtrim($themePath, '/') . '/web/tailwind';
157157

158-
// Check for node_modules directory
159-
if (!$this->fileDriver->isDirectory($tailwindPath . '/node_modules')) {
158+
// Check if node_modules is in sync with package-lock.json
159+
if (!$this->nodePackageManager->isNodeModulesInSync($tailwindPath)) {
160160
if ($isVerbose) {
161-
$io->warning('Node modules not found in tailwind directory. Installing...');
161+
$io->warning('Node modules out of sync or missing. Installing dependencies...');
162162
}
163163
if (!$this->nodePackageManager->installNodeModules($tailwindPath, $io, $isVerbose)) {
164164
return false;

‎src/Service/ThemeBuilder/TailwindCSS/Builder.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,10 @@ public function autoRepair(string $themePath, SymfonyStyle $io, OutputInterface
126126
{
127127
$tailwindPath = rtrim($themePath, '/') . '/web/tailwind';
128128

129-
// Check for node_modules directory
130-
if (!$this->fileDriver->isDirectory($tailwindPath . '/node_modules')) {
129+
// Check if node_modules is in sync with package-lock.json
130+
if (!$this->nodePackageManager->isNodeModulesInSync($tailwindPath)) {
131131
if ($isVerbose) {
132-
$io->warning('Node modules not found in tailwind directory. Installing npm dependencies ...');
132+
$io->warning('Node modules out of sync or missing. Installing npm dependencies...');
133133
}
134134
if (!$this->nodePackageManager->installNodeModules($tailwindPath, $io, $isVerbose)) {
135135
return false;

0 commit comments

Comments
 (0)