Skip to content

Replace the Grunt toolchain with npm scripts, phpcs and CI - #111

Merged
puikinsh merged 3 commits into
masterfrom
tooling/modernize
Aug 7, 2026
Merged

Replace the Grunt toolchain with npm scripts, phpcs and CI#111
puikinsh merged 3 commits into
masterfrom
tooling/modernize

Conversation

@puikinsh

@puikinsh puikinsh commented Aug 7, 2026

Copy link
Copy Markdown
Member

Tooling only. No shipped file changes, no version bump, nothing users see. fancybox.php and readme.txt are untouched at 3.4.1.

Why

The Grunt setup wasn't broken — I ran it, and it produced a byte-identical package. But it pulled 293 packages and 5 vulnerabilities (2 high: braces and micromatch ReDoS, both via grunt-cli) to do one job — copy files and zip them — while none of the tasks that actually caused friction this month were covered.

before after
direct devDependencies 8 2
packages installed 293 34
npm audit 5 (2 high) 0
deprecated transitives 6 2 (dev-only, via clean-css)

What's automated that wasn't

npm run build:assets    minification — previously ad-hoc `npx terser` calls
npm run verify:assets   fails when a .min file is stale
npm run i18n            .pot / .mo / .l10n.php via wp-cli
npm run package         the zip, plus version + dev-file checks
composer phpcs          escaping, sanitisation, nonces, i18n, PHP 7.4–8.5
composer lint           php -l over every file

Two guards that encode mistakes already made

verify:assets rebuilds each minified file and byte-compares. The .min files are what visitors actually get — mfbfw_asset_suffix() only serves the readable sources under SCRIPT_DEBUG — so editing fancybox.css without rebuilding would silently ship the old stylesheet. Confirmed it exits 1 when tripped.

package refuses to build unless the plugin header, FBFW_VERSION, the readme Stable tag and package.json all agree. That's the exact drift that left package.json at 3.3.5 for three releases.

The release workflow is the important one

.github/workflows/deploy.yml publishes to WordPress.org SVN on a version tag. 3.3.7 shipped to WordPress.org and was never committed here, and 3.4.1 shipped while master still said 3.4.0. Releasing from a tag makes git the source of truth and removes the manual step where both happened.

⚠️ It needs SVN_USERNAME and SVN_PASSWORD repository secrets before it can run. Until those exist, the deploy job simply won't fire; CI is unaffected.

On phpcs being a subset

phpcs.xml.dist deliberately does not run the full WordPress standard. I ran it first: 293 violations, every single one whitespace, array alignment or a missing docblock — and zero security, i18n or compatibility findings. Enforcing formatting would mean reflowing ~250 lines across a plugin with 30,000 installs for no functional gain, and would make every future diff harder to read.

The rules kept are the ones that catch real defects — WordPress.Security, WordPress.WP.I18n, PHPCompatibilityWP and friends. All of them fired on 3.3.7 and were fixed in 3.4.0. They pass clean today: 0 errors across all 11 PHP files. The ruleset documents how to adopt full formatting later via phpcbf in its own commit.

Also

.distignore becomes the single source of truth for packaging — bin/package.mjs and the deploy action both feed it to rsync --exclude-from. Gruntfile.js carried a second exclude list that had to be hand-synced, and is removed.

package-lock.json was in .gitignore; CI installs with npm ci, which cannot run without it. Both lockfiles are now committed.

Verified

  • A fresh clone running npm ci && npm run package produces a zip byte-identical to the published 3.4.1
  • phpcs: 0 errors, 0 warnings across all 11 PHP files
  • composer lint: clean
  • Both guards exit non-zero when tripped

CI will run on this PR, so the workflows get exercised before they're relied on.

🤖 Generated with Claude Code

puikinsh and others added 2 commits August 7, 2026 11:55
Tooling only. No shipped file changes, no version bump, nothing for users.

The Grunt setup still worked - it produced a byte-identical package - but it
pulled 293 packages and 5 vulnerabilities (2 high: braces and micromatch ReDoS,
both via grunt-cli) to do one job, copy files and zip them, while none of the
tasks that actually caused friction were covered.

  before: 8 direct deps -> 293 packages, 5 vulnerabilities
  after:  2 direct deps ->  34 packages, 0 vulnerabilities

What is now automated that was not:

  npm run build:assets   minification, previously ad-hoc `npx terser` invocations
  npm run verify:assets  fails when a .min file is stale
  npm run i18n           .pot / .mo / .l10n.php via wp-cli
  npm run package        the zip, plus version and dev-file checks
  composer phpcs         escaping, sanitisation, nonces, i18n, PHP 7.4-8.5
  composer lint          php -l over every file

Two guards encode mistakes that already happened. verify:assets rebuilds each
minified file and byte-compares, because the .min files are what visitors
actually get - editing fancybox.css without rebuilding would silently ship the
old stylesheet. package refuses to build unless the plugin header, FBFW_VERSION,
the readme Stable tag and package.json agree, which is exactly the drift that let
package.json sit at 3.3.5 for three releases.

.distignore becomes the single source of truth for packaging. bin/package.mjs and
the deploy workflow both feed it to `rsync --exclude-from`, so a local build and a
published release contain the same files. Gruntfile.js carried a second exclude
list that had to be hand-synced, and is removed.

.github/workflows/deploy.yml publishes to WordPress.org SVN on a version tag. This
is the important one: 3.3.7 shipped to WordPress.org and was never committed here,
and 3.4.1 shipped while master still said 3.4.0. Releasing from a tag makes git
the source of truth and removes the manual step where both happened. It needs
SVN_USERNAME and SVN_PASSWORD repository secrets before it can run.

phpcs.xml.dist deliberately runs a subset of the WordPress standard. The full
ruleset reports 293 violations on this codebase, every one whitespace, array
alignment or a missing docblock, and zero security, i18n or compatibility
problems. Enforcing formatting would mean reflowing ~250 lines across a plugin
with 30k installs for no functional gain, so the rules kept are the ones that
catch real defects - and they pass clean today. The file documents how to adopt
full formatting later.

Also un-ignores package-lock.json, which .gitignore excluded: CI installs with
`npm ci`, which cannot run without it.

Verified: a fresh clone running `npm ci && npm run package` produces a zip
byte-identical to the published 3.4.1, phpcs reports 0 errors across all 11 PHP
files, and both guards exit non-zero when tripped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The job failed on its first run with a single error:

  FILE: phpcs.xml.dist
  ERROR application_detected: Application files are not permitted.

Pointing the action at the repository root makes it judge files that never ship -
phpcs.xml.dist, bin/, CLAUDE.md - and says nothing about what users install. It
now builds the distributable first and checks that, which is both correct and the
thing worth gating on.

The directory name has to be the plugin slug: the text-domain check compares
against it, which is why `npm run package` staging into
build/fancybox-for-wordpress matters.

Verified locally against the built package: 0 errors, and the only warnings are
the known long-standing ones - unprefixed globals kept for backwards
compatibility, "WordPress" in the grandfathered plugin name, and
load_plugin_textdomain, which is required while translations are bundled.

Also grants the job pull-requests: write so the action can post its findings as a
PR comment; it warned "Resource not accessible by integration" without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.


🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

Neither SVN_USERNAME nor SVN_PASSWORD is configured on this repository yet, so
the first tag push would have died deep inside the 10up deploy action with an
authentication error that does not say what is actually wrong.

The job now checks up front and names the missing secrets, where to add them, and
the fact that the tag is already pushed so the workflow can simply be re-run once
they exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@puikinsh
puikinsh merged commit e15a71d into master Aug 7, 2026
9 checks passed
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.

1 participant