Skip to content

build: clear all Dependabot alerts and migrate the asset pipeline to Vite - #86

Open
Loubal70 wants to merge 7 commits into
developfrom
feat/83923_dependatbot
Open

Loubal70 wants to merge 7 commits into
developfrom
feat/83923_dependatbot

Conversation

@Loubal70

@Loubal70 Loubal70 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Clears all 60 Dependabot alerts, retires the dead build toolchain they came from, and fixes an asset leak found along the way.

Five commits, reviewable independently.


1. fix(deps): patch 58 Dependabot security alerts

Source Alerts
yarn.lock (build toolchain) 56
GitHub Actions (setup-php) 4
composer.lock (shipped PHP) 0

None of these ever reached merchants: node_modules is in .distignore, so nothing from the npm tree ships to WordPress.org. The only npm package bundled for the browser is Alpine.js, which has no alerts.

  • shivammathur/setup-php 2.37.0 → 2.37.2 in both deploy workflows
  • Recursive upgrade of every vulnerable transitive dependency

Two alerts resisted: elliptic (no patched release exists — 6.6.1 is the latest and is itself vulnerable) and immutable (capped at 3.x by browser-sync@3.0.4, itself the latest release). Both are resolved by the next commit.

2. build: migrate the asset pipeline from Laravel Mix to Vite

Laravel Mix has had no release since June 2022 and still pulls node-libs-browser, the webpack 4 polyfill package — which is exactly where elliptic came from. browser-sync is what capped immutable. Both alerts disappear along with those dependencies.

Before After
Packages in node_modules 861 507
Open CVEs 60 alerts 0
resolutions entries 6 2
Node 16 (.nvmrc) / 18 (CI) 24 LTS everywhere

Node had to move: Vite 8 requires ^20.19 || >=22.12, so the previous Node 18 in CI could not have run the build at all.

No PHP was touched in this commit. It only changes how dist/ is produced, not how WordPress loads it.

Two things worth a close look in vite.config.mjs:

  • The IIFE wrapper. Rollup rejects iife for multi-entry builds (Multiple entry points are not supported when output formats include "umd" or "iife"), so the output is ESM. Loaded through wp_enqueue_script() as a classic script, its top-level declarations land on window — a minified _ clobbered WordPress's Underscore and broke wp.media with _.isArray is not a function. The bundle is therefore wrapped through banner/footer. type="module" would be cleaner but defers execution, and c24c711 (PLG-264) fixed precisely that class of bug: the consent signal arriving after a hardcoded GTM tag had already fired. Deferring axeptio.js would have reintroduced it.
  • @tailwind@import in assets/css/backend/main.css. The CSS spec requires @import to precede any other rule; the directives came first, so the bundler silently dropped every @import and stripped all custom rules (8 KB of buttons, tabs, forms, notices). Rewriting them as @import "tailwindcss/base" keeps cascade order identical.

The four entry points mirror how WordPress enqueues them: admin and frontend are separate contexts, and merging them would ship Alpine.js and Tailwind (118 KB) on every public page of every merchant site.

3. fix(admin): stop loading the plugin stylesheet on every admin page

Unrelated to the migration, found while verifying it — and pre-existing since 2023.

admin_scripts() has been guarded by a screen check since 4177b73; admin_styles() never was. backend/main.css (51 KB) was enqueued on every admin screen — Posts, Media, Settings, Users — for a stylesheet only two pages use.

The check now lives in is_plugin_admin_screen(), called by both, so the two lists cannot drift apart again. It takes the $hook_suffix WordPress already passes to admin_enqueue_scripts, which removes the get_current_screen() call the old code relied on (and the null guard it required).

Verified beforehand that nothing else depends on that stylesheet: the review banner renders through axeptio/before_main_setting_container and axeptio/before_plugin_manager_container, both internal to those two pages, and the plugin's only admin_notices callback uses WordPress's native notice notice-error classes.


Verification

Everything below was run inside ddev (Linux, Node 24), matching CI.

Build chain

  • yarn install --immutable — passes, the lockfile is CI-clean
  • yarn eslint — exit 0
  • yarn build:productiondist/ identical to the Mix output except three unused artifacts (*.LICENSE.txt, mix-manifest.json), none referenced by PHP
  • CSS: 502 of 503 selectors identical; the only difference is a -moz- prefix obsolete since Firefox 51
  • phpcs, phpstan, pest on includes/core.php — clean
  • Packaging simulated from .distignore: vite.config.mjs, package.json, yarn.lock, node_modules excluded; dist/, includes/, vendor/ included

Admin asset loading — 10 pages swept

Page CSS JS
Axeptio — Settings loaded loaded
Axeptio — Plugin manager loaded loaded
Dashboard, Posts, Media, Pages, Plugins, Settings, Users, Tools

Frontend — consent path unchanged

Mix Vite
Consent Mode snippet, synchronous in <head>
axeptio.js position in page script 14 of 18 14 of 18
Load order wp-consent-api → inline → axeptio.js identical
Axeptio_SDK, _axcb, wp_consent_type, steps, vendors, categories all set all set
dataLayer entries 10 10

Rendering — Alpine reactive, tab switching works, 502 CSS rules applied, wp.media functional, review banner intact, images and fonts resolve.

4. docs + build — follow-ups from the Copilot review

  • docs: update the build stack and Node version in the environment matrix and the docs fixes that followed. environments.md had been updated but workflows.md and improvements.md still documented Node 18 and setup-php@2.37.0, contradicting the workflow changes in this PR. No stale version or webpack mention is left under docs/.
  • build: copy the static assets on every bundle writecloseBundle becomes writeBundle, which is the correct hook for a copy that must follow every write.

Two review points were investigated and are worth recording:

  • Copilot reported that closeBundle never runs during a watch build, leaving dist/img and dist/fonts missing after a clean yarn start. Testing does not support this: rm -rf dist && yarn start produces both directories (10 images, 1 font). The hook change above was made on correctness grounds, not to fix that.
  • Copilot also reported that the static directories are outside Rollup's watch graph, so editing an image alone does not refresh its copy. That one is accurate — confirmed by test: editing alert.svg alone changes nothing, editing any CSS afterwards refreshes it. The suggested fix does not work here, however: addWatchFile was implemented both on the directories and file-by-file through a recursive readdirSync, and neither triggered a rebuild under Vite 8 / Rolldown — the log shows a single built in. The dead code was removed rather than left in place looking functional, and the limitation is documented in the config. In practice, editing an image requires touching any JS or CSS for the copy to refresh.

Impact on the pipelines

Every consumer of the npm scripts was audited, since dev, production and hot were removed along with webpack.

Pipeline Uses Status
deploy.yml (WordPress.org) corepack enable, yarn install --immutable, yarn build:production ✅ all three rehearsed in ddev; Node bumped to 24 in the workflow
deploy-demo.yml (QA demo) same chain ✅ same, Node bumped to 24
Taskfile.yml (task release) yarn && yarn build:production ✅ script kept; exclusions.txt updated alongside .distignore
docs/deployment/* referenced webpack watch and Node 18 ✅ updated in the fourth commit

One pipeline needs a decision: .gitlab-ci.yml. Its eslint job runs image: node:16.14.0, then yarn && yarn eslint.

That job is already failing today, before this PR: packageManager: "yarn@4.9.2" was added in 46a3121, and the Yarn 1.22 shipped in the Node 16 image refuses to run on a project declaring it. Yarn 4 itself requires Node >= 18.12, so the image was never going to work.

This PR does not make it worse, but it does close the door for good: Vite 8 requires Node ^20.19 || >=22.12. If that pipeline is still wired to anything, the image needs to move to node:24 and the job to run corepack enable first. If it is a leftover from the pre-GitHub days, it should be deleted. I left it untouched — that call is yours.

Notes for review

  • Dependabot tracks the default branch (main), so merging into develop will not close the alerts — they clear once this reaches main.
  • ddev needs corepack_enable: true (in .ddev/config.yaml, outside this repo). Without it the container falls back to Yarn 1.22 and yarn install fails on the packageManager field. CI already runs corepack enable.
  • node_modules is shared between host and container through the ddev mount, and Rolldown ships platform-specific native bindings — so running yarn install on the host then building in the container (or the reverse) breaks. Pick one environment.
  • No workflow builds on pull requests: deploy.yml runs on tags, deploy-demo.yml on pushes to develop. This PR is therefore not exercised by CI before merge. A small ci.yml (install + lint + build on PRs) would close that gap — happy to add it here or separately.
  • The compiled assets are not byte-identical to the previous release. Local checks cover the admin and the front on ddev, but the widget should be validated on a real site before this reaches main: SDK injection, consent handling, and Google Consent Mode signals.

Copilot AI lite review requested due to automatic review settings September 10, 2026 13:11
@axeptio-bot
axeptio-bot self-requested a review September 10, 2026 13:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Broad toolchain changes require additional validation of front-end widget and consent behavior.

Pull request overview

Patches 58 Dependabot alerts in build-only dependencies and GitHub Actions without changing shipped PHP dependencies.

Changes:

  • Upgrades vulnerable Yarn dependencies and adds four targeted resolutions.
  • Updates setup-php in deployment workflows.
  • Leaves two documented local-tooling alerts unresolved.
File summaries
File Summary
yarn.lock Updates vulnerable build dependencies and checksums.
package.json Adds dependency resolutions.
.github/workflows/deploy.yml Updates the PHP setup action.
.github/workflows/deploy-demo.yml Updates the PHP setup action.
Review details
  • Files reviewed: 3/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

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

@Loubal70
Loubal70 marked this pull request as draft September 10, 2026 13:17
@Loubal70 Loubal70 changed the title fix(deps): patch 58 Dependabot security alerts build: clear all Dependabot alerts and migrate the asset pipeline to Vite Sep 10, 2026
@Loubal70
Loubal70 force-pushed the feat/83923_dependatbot branch from 2bf9d06 to 587f3a6 Compare September 10, 2026 14:46
@Loubal70
Loubal70 marked this pull request as ready for review September 10, 2026 15:04
Copilot AI review requested due to automatic review settings September 10, 2026 15:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved Vite asset-copying, GitLab CI compatibility, and deployment-documentation issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

vite.config.mjs:11

  • These cpSync calls do not add the static directories to Rollup's watch graph. yarn start therefore does not rebuild when a PHP-only asset such as assets/img/icon.png or logo.svg changes, leaving the corresponding dist copy stale until another entry rebuild or a production build. Register the static files/directories with this.addWatchFile or use a copy plugin with watch support.
		cpSync('assets/img', 'dist/img', { recursive: true });
		cpSync('assets/fonts', 'dist/fonts', { recursive: true });

vite.config.mjs:9

  • closeBundle is not run after each Rollup watch rebuild; it is deferred until the watcher shuts down. A clean checkout followed by yarn start therefore never copies dist/img or dist/fonts, so the admin icon and font/image references are missing during development. Use writeBundle for this copy (and watch the static directories if live changes are expected).
	closeBundle() {
  • Files reviewed: 13/15 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread package.json
Comment thread docs/deployment/environments.md
Copilot AI review requested due to automatic review settings September 10, 2026 15:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Unresolved moderate findings remain for the legacy GitLab CI job and packaging exclusions.

Review details

Suppressed comments (2)

docs/deployment/improvements.md:46

  • This table is explicitly the historical record for PRs #47/#48/#50, but these rows now attribute this PR's Node 24 and setup-php 2.37.2 upgrades to those older PRs. Keeping the historical Node 18/2.37.0 values here and documenting the current migration separately avoids making the changelog inaccurate.
| 2   | Workflow | Node.js version mismatch: CI used Node 16; `.nvmrc` and `volta` pin Node 24    | Node 24 now used in CI; Node 16 was EOL since September 2023                        |
| 4   | Workflow | Outdated GitHub Actions: `checkout@v2`, `setup-node@v2`                        | Upgraded: `checkout@v4`, `setup-node@v4`, `setup-php@2.37.2`                        |

package.json:29

  • This new engine floor leaves the repository's .gitlab-ci.yml ESLint job on node:16.14.0 (and it still invokes the bundled Yarn 1 without corepack enable), so any GitLab pipeline that is still enabled will fail before linting. Update that job to Node 24 with Corepack/Yarn 4, or remove the legacy workflow if it is truly retired.
    "node": ">= 22.12.0"
  • Files reviewed: 15/17 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 10, 2026 15:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Address the missing PR build check and exclude Playwright artifacts from both release packaging lists.

Review details

Suppressed comments (1)

package.json:44

  • This new Vite build is still not exercised by any pull-request workflow: deploy.yml runs on tags/manual dispatch and deploy-demo.yml runs on develop, so a broken install or production build can merge without CI feedback. Please add a PR check covering Corepack, the immutable install, lint, and build:production, or make an equivalent required check part of this migration.
    "vite": "^8"
  • Files reviewed: 15/17 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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