JS asset pipeline reorders failed assets, swallows Errors, and never reads its cache
Summary
The JS pipeline has the same three problems the CSS pipeline is being fixed for in #4307, and one of them is worse on the JS side.
Pipeline::gatherAndMinifyJs() minifies each file individually, excludes any that throw from the combined buffer, and returns them in failed. Assets::render() then appends every failed asset after the whole bundle:
https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Assets.php#L481-L488
For CSS that changes which rule wins. For JS it changes execution order. If the file the minifier chokes on is a dependency — jQuery, a polyfill, anything that defines a global — it now loads after everything that expects it, and the page dies with a ReferenceError it did not have before. The failure is silent at build time and only shows up in the browser.
Three separate problems
1. Failed assets execute out of order. As above. The fix is the same one #4307 lands for CSS: when any asset in the group fails to minify, bundle the whole group unminified in its original order rather than emitting a partial bundle plus a reordered tail.
2. catch (\Exception) lets \Error through.
https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Assets/Pipeline.php#L415
A TypeError out of JShrink is an \Error, not an \Exception, so it escapes the catch and 500s the page — which is the exact crash the per-asset isolation was added to prevent. #4307 correctly uses catch (\Throwable) for CSS; JS should match.
3. A group with one bad file re-minifies on every request.
https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Assets/Pipeline.php#L243-L248
renderJs() only reads the cache when there are no failures (if (empty($failedAssets) && file_exists($filepath))), but writes the partial bundle every time. So a site with a single unminifiable JS file runs JShrink over its entire JS group on every page view, forever, and never reads the file it just wrote.
Suggested fix
Mirror whatever lands in #4307 for CSS, plus widen the catch:
- On any failure in the group, fall back to
gatherLinks()for the whole group so order is preserved and the bundle still caches. catch (\Exception $e)→catch (\Throwable $e).
Found while reviewing #4307. Filing separately so the CSS fix isn't held up.
Source: getgrav/grav