Skip to content

Fix build tooling and modernize WPCS integration - #109

Open
lihsaa591 wants to merge 7 commits into
masterfrom
chore/build-tooling-and-phpcs
Open

lihsaa591 wants to merge 7 commits into
masterfrom
chore/build-tooling-and-phpcs

Conversation

@lihsaa591

Copy link
Copy Markdown
Contributor

Summary

Fixes a completely broken build (missing dependencies, PHP 8 crash) and modernizes/repairs the existing (broken) WPCS setup for the Flash theme.

Changes

Area Change Why
package.json Added grunt-cli@^1.4.3 to dependencies Never declared at all — npm run makepot/compress failed with grunt: command not found on a clean install regardless of any other fix
package.json Added gulp-rename@^2.1.0 to devDependencies gulpfile.js requires it, but it was never declared — npm run styles failed with Cannot find module 'gulp-rename'
package.json stylelint-config-wordpress ^12.0.0 → ^13.0.0 Pre-existing conflict: stylelint@^9.2.1 (already declared) satisfies neither stylelint-config-wordpress@12's peer (^7.10.1) nor gulp-stylelint@^7.0.0's peer (^9.0.0) at the same time. ^13.0.0 requires stylelint@^9.1.3, compatible with both, fixing the ERESOLVE failure with the smallest possible change
package.json grunt 0.4.5 → ^1.6.3, grunt-contrib-compress ^1.2.0 → ^2.0.0, grunt-contrib-jshint ~0.11.3 → ^3.2.0, grunt-contrib-uglify ^0.10.1 → ^5.2.2, grunt-contrib-watch ~0.6.1 → ^1.1.0 Years out of date; bumped in step even though most of these grunt tasks aren't wired into npm run build (styles/uglify go through gulp) — they're still unconditionally loaded at Gruntfile.js init and would break makepot/compress too if they failed to load
package.json grunt-wp-i18n ~0.5.3 → ^1.0.4 Old version's bundled makepot.php calls the removed PHP 8 function create_function(), fatal-erroring grunt makepot
package.json engines.node/engines.npm >=8.9.3/>=5.5.1 → >=20.0.0/>=9.0.0 grunt-wp-i18n@1.0.4's locked node-wp-i18n@1.2.8 → glob@11.1.0 requires Node 20 or ≥22
Gruntfile.js compress task now also excludes composer.json, composer.lock These were being archived straight into the release zip (root-level Kirki's own bundled composer.json/kirki-composer/ autoloader under inc/kirki/ is untouched — that's genuinely needed at runtime)
Gruntfile.js makepot task now sets potComments explicitly grunt-wp-i18n@1.0.4's default comment template leaks a literal <!=...=!> placeholder into the .pot header when not overridden — an upstream bug
Gruntfile.js uglify task uses compress.arrows: false and output.comments instead of preserveComments Same fix as the sibling themes — preserveComments was removed in grunt-contrib-uglify 2.x+, and the new major emits ES2015 arrows by default. (This grunt task isn't wired into npm run build currently — gulp-uglify handles the shipped js/*.min.js, and produces clean ES5 with no changes needed — but fixed for anyone running grunt default/grunt dev directly)
.npmrc (new) omit=optional Without it, npm install silently skips all devDependencies whenever the shell has NODE_ENV=production set
composer.json squizlabs/php_codesniffer 3.2 → ^3.9, wp-coding-standards/wpcs 0.14.0 → ^3.1, wimg/php-compatibility → phpcompatibility/phpcompatibility-wp ^2.1, dealerdirect/phpcodesniffer-composer-installer ^0.7.2 → ^1.0; added phpcs/phpcbf scripts Existing versions predate current PHPCS/WPCS entirely and don't even run cleanly against them (see next row)
phpcs.xml text_domain was "cenote" — fixed to "flash" Copy-paste leftover from a different ThemeGrill theme; this theme's actual text domain (confirmed in style.css) is flash
phpcs.xml Removed WordPress.VIP.RestrictedFunctions exclude and the whole WordPress.WP.DeprecatedFunctions override Both reference sniffs/properties removed in modern WPCS — PHPCS refused to run at all (ERROR: Referenced sniff ... does not exist), not just report stale findings
phpcs.xml testVersion 5.2-99.0 → 7.4-, PHPCompatibility → PHPCompatibilityWP Old package name/ruleset; theme's actual minimum is PHP 7.4
phpcs.xml Added exclude-pattern for inc/kirki/* Bundled third-party Kirki library — not this theme's code, and previously being linted as if it were (3707 → 593 baseline errors after excluding it)
.github/workflows/phpcs.yml (new) Runs PHPCS on PR, scoped to added/copied/modified/renamed PHP files only (not deleted) ~593 pre-existing errors across untouched theme files — full-repo linting would block on unrelated debt, and including deleted files would fail the job on any PR that removes a PHP file
languages/flash.pot Regenerated via npm run build on the fixed toolchain Output of the now-working build, not a manual edit

QA / Test steps

Toolchain (no WordPress needed):

  1. git clone this branch fresh, cd into it.
  2. Run npm install with no flags.
    • Expected: completes with no ERESOLVE error, and both node_modules/.bin/grunt and node_modules/.bin/gulp exist.
  3. Run npm run build.
    • Expected: styles (gulp/Sass), uglify (gulp), makepot (grunt), and compress (grunt) all complete, exit code 0. No Cannot find module 'gulp-rename', no grunt: command not found, no create_function() PHP fatal error.
  4. Open languages/flash.pot and check the first 2 lines.
    • Expected: # Copyright (C) <year> ThemeGrill / # This file is distributed under the GNU General Public License, version 3 (GPLv3). — plain text, no <!= or =!> characters.
  5. Run grep -c '=>' js/*.min.js.
    • Expected: 0 for every file — no ES2015 arrow functions in the minified output.
  6. Run unzip -l dist/flash.zip | grep -E 'flash/(composer\.(json|lock)|phpcs\.xml)$'.
    • Expected: no output — none of this repo's own tooling files leaked into the release zip (Kirki's own nested inc/kirki/composer.json etc. are expected to remain — that's runtime code, not our tooling).
  7. Run composer install && composer phpcs.
    • Expected: runs cleanly against the fixed phpcs.xml (previously errored out entirely with "Referenced sniff ... does not exist"). Reports the pre-existing ~593-error backlog — expected and out of scope for this PR.

Theme behavior in WordPress (manual — do this before merging):

  1. Take dist/flash.zip from step 3 above (or activate the theme directory directly) and activate Flash on a WordPress install.
  2. Visit the homepage, a blog archive, and a single post — confirm layout, fonts, and icons render as before.
  3. Open the Kirki-powered Customizer panels and confirm they still load and save (Kirki itself wasn't touched, but its composer autoloader lives right next to the files this PR's compress-exclude change touches).
  4. If translations are in use, confirm Loco Translate (or similar) can still read languages/flash.pot without errors.

Changelog entry

Fix - Fixed a broken npm build (missing dependencies) and modernized/repaired the theme's coding-standards tooling.

🤖 Generated with Claude Code

lihsaa591 and others added 2 commits September 22, 2026 14:23
grunt-cli and gulp-rename were used by the Gruntfile/gulpfile but
never declared as dependencies, so `npm run makepot`/`compress` and
`npm run styles` failed outright on a clean install regardless of the
grunt/gulp versions installed. Also bumps grunt and grunt-contrib-*
packages, fixes compress excluding composer files from the release
zip, fixes makepot's PHP 8 create_function() crash, and keeps Uglify
output ES5 with license headers preserved.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
phpcs.xml's text_domain was set to "cenote" -- a different ThemeGrill
theme, clearly copy-pasted in -- and referenced sniffs/properties
(WordPress.VIP.RestrictedFunctions, DeprecatedFunctions'
minimum_supported_version) removed in modern WPCS, which made phpcs
error out entirely rather than just report stale findings. Fixed the
text domain, dropped the obsolete rules, excluded the bundled
third-party Kirki library from linting, and bumped composer.json to
current squizlabs/wpcs/phpcompatibility-wp versions with phpcs/phpcbf
scripts. Adds a PR workflow that lints changed PHP files only
(added/copied/modified/renamed, not deleted).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The new PHPCS workflow uses an incorrect PHP glob (**.php) and the makepot header/template has a malformed language-team value, both of which should be corrected before merging.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

This PR repairs and modernizes the Flash theme’s build/tooling pipeline (npm/grunt/gulp) and updates the PHP coding standards setup (Composer + WPCS/PHPCS), including adding a CI PHPCS workflow and regenerating the theme POT file.

Changes:

  • Updated Node/Grunt/Gulp/Stylelint dependency versions and added missing JS build dependencies to restore a working npm run build.
  • Modernized PHPCS/WPCS Composer tooling and updated phpcs.xml configuration (including PHPCompatibilityWP and exclusions).
  • Added a GitHub Actions workflow to run PHPCS on changed PHP files and regenerated languages/flash.pot.
File Description
phpcs.xml Updates ruleset, text domain, PHPCompatibilityWP config, and exclusions.
package.json Fixes missing deps and modernizes build tool versions; raises Node/npm engines.
Gruntfile.js Updates uglify options, POT header template, and zip exclusions.
languages/​flash.pot Regenerated POT output from the updated toolchain.
composer.json Updates PHPCS/WPCS-related dev dependencies and adds phpcs/phpcbf scripts.
composer.lock Locks updated PHPCS/WPCS toolchain dependency graph.
.npmrc Adds npm install behavior override (omit=optional).
.github/​workflows/​phpcs.yml Adds PR-time PHPCS checks scoped to changed PHP files.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +4 to +6
pull_request:
paths:
- '**.php'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked this against both tools' own docs rather than changing it on faith: GitHub's own path-filter syntax docs give **.js as the documented example for "all .js files in the repository" (i.e. it already matches nested paths, no leading **/ needed) — same semantics apply to **.php in the on.pull_request.paths filter. And tj-actions/changed-files' own README (the files: input used in the second step) has an identical example, **.md, specifically captioned "Get all changed markdown files" including nested ones under docs/ — it explicitly adopted GitHub's native glob semantics as of v13, not minimatch's. So **.php already matches nested PHP files under both tools as written. Switching to **/*.php would be redundant at best, and in some glob dialects **/*.php actually fails to match root-level .php files (no directory segment before the file) — which would be a real regression for a WordPress theme, where most template files (front-page.php, single.php, etc.) live at the theme root. Leaving as-is unless you have a specific counter-example of a nested file this pattern misses.

Comment thread Gruntfile.js
Comment on lines +102 to 105
potComments: 'Copyright (C) {year} ThemeGrill\nThis file is distributed under the GNU General Public License, version 3 (GPLv3).',
potHeaders: {
'report-msgid-bugs-to': 'themegrill@gmail.com',
'language-team': 'ThemeGrill <themegrill@gmail.com'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 50a7868 — makepot's exclude list only had deploy/.; added inc/kirki/. alongside it. Regenerated the .pot: Kirki-related string count went from 217 matches to 0, confirmed with grep -ci kirki languages/flash.pot before/after.

@lihsaa591 lihsaa591 self-assigned this Sep 22, 2026
Supersedes the stalled PR #105, which used yarn (this repo has never
used yarn -- no yarn.lock exists, npm/package-lock.json is what's
actually used) and predates this branch's PHPCS/composer.json setup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
lihsaa591 and others added 2 commits September 22, 2026 14:34
package-lock.json isn't committed here (matches accelerate-pro's own
.gitignore convention), so npm ci has no lockfile to install from and
fails with EUSAGE.

#build-zip

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tg-autopilot

Copy link
Copy Markdown
Contributor

Build for 55c977c0 is ready 🛎️

⬇️ Download flash.zip (3.1M)

Installs directly via Plugins → Add New → Upload Plugin.
Link expires in 30 days · updated Sep 22, 2026 2:38 PM +0545

A later push only rebuilds this if its commit message includes #build-zip.

lihsaa591 and others added 2 commits September 22, 2026 14:41
makepot only excluded deploy/, so third-party Kirki UI/library
strings (font weights, control labels, etc.) were leaking into
languages/flash.pot alongside this theme's own translatable strings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
style.css declares Requires PHP: 5.6, but testVersion was set to
7.4- -- silently raising the theme's real compatibility contract
without that being a deliberate product decision. Set to 5.6- to
match what's actually declared; bumping the real minimum is a
separate call for the team to make, not something to smuggle into
tooling setup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

3 participants