feat: minimal SS5 compatibility upgrade#252
Conversation
- Update dependencies for SS5 (recipe-cms ^5.0, lumberjack ^3.0, etc.) - Remove deprecated muskie9/data-to-arraylist dependency - Replace DataToArrayListHelper with ArrayList::create() - Remove phpcoverage: false from CI workflow - Add jQuery CDN for frontend (SS5 admin bundles jQuery)
There was a problem hiding this comment.
Pull request overview
Updates the module for SilverStripe 5 compatibility by modernising dependencies and removing reliance on SS4 admin-provided jQuery / third-party helper utilities.
Changes:
- Update Composer requirements to SilverStripe CMS Recipe v5–compatible module versions and adjust dev tooling.
- Replace
muskie9/data-to-arraylistusage with coreArrayListconversion. - Update frontend requirements to load jQuery differently and simplify CI workflow inputs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| templates/Dynamic/Locator/Page/Layout/Locator.ss | Switches jQuery requirement source and keeps store-locator JS requirements. |
| src/Page/LocatorController.php | Removes muskie9/data-to-arraylist dependency and converts locations to an ArrayList using core APIs. |
| composer.json | Bumps dependency constraints to SS5-compatible majors; adds dev dependency for PHPCS. |
| .github/workflows/ci.yml | Removes the explicit phpcoverage: false workflow input. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,5 @@ | |||
| <% require css('dynamic/silverstripe-locator: css/map.css') %> | |||
| <% require javascript('https://code.jquery.com/jquery-3.7.1.min.js') %> | |||
There was a problem hiding this comment.
Requiring jQuery directly from a public CDN introduces an external runtime dependency for the frontend (can break in environments with strict CSP/no-internet access), and it bypasses SilverStripe’s asset management (no version control within the module, no easy pinning/auditing). Consider vendoring a compatible jQuery build into this module (e.g. under thirdparty/ or client/) and requiring it via the vendormodule resource path, or otherwise documenting/expecting the host project to provide jQuery (and avoid loading a second copy here).
| <% require javascript('https://code.jquery.com/jquery-3.7.1.min.js') %> | |
| <% require javascript('dynamic/silverstripe-locator: thirdparty/jquery/jquery-3.7.1.min.js') %> |
No description provided.