Contrib.social

The Great List of Refactoring Messy Code Things™

Open on Drupal.org →
Created on 4 March 2022, over 2 years ago
Updated 13 September 2023, about 1 year ago

So after getting my hands metaphorically dirty with #3060760: Use Google Workbox library → , I've noticed quite a few things that can be refactored to make the codebase less...let's say painful to work with? This issue will be an ongoing attempt at documenting and planning this stuff out, so expect many edits and additions.

I think a lot of this could be backported in 8.x-1.x, but part of me wonders if the time spent merging stuff into both branches might not be better spent focusing entirely on 2.x. Discuss in comments.

Goals

  • Partially split off huge PHP files that are a catch-all for a lot of things (e.g. PWAController).
  • Modernize the codebase with modern PHP language features.

Specifics

  • \pwa_str_to_list() in pwa.module; this is used in many places to convert strings from the settings forms into arrays and back again.
    1. One option is to convert this to a utility class and just search and replace.
    2. Another option is to make this more like a value object of some sort that you instantiate and be cast to a string.
    3. Lastly, we could look at the problem it's solving and see if we can remove the problem entirely. In the Workbox port, I've already converted a few configs stored as strings into sequences or maps, so instead of having textareas that we have to parse, we could implement form UI with multiple rows and buttons or links to add an additional row.
  • \pwa_page_attachments() and \pwa_library_info_alter() (latter added in #3060760: Use Google Workbox library → ): these could be refactored as services so that the procedural hooks only have to call a single thing, thus allowing better use of dependency injection and a future move to object-oriented hooks, which is currently possible via Hook Event Dispatcher → or when core finally implements them (see 📌 Delegate all hook invocations to ModuleHandler Fixed for some ongoing work).
  • \Drupal\pwa\Controller\PWAController:
    1. PWAController::fetchOfflinePageResources(): this can be moved to its own service that's injected into PWAController and where ever else we may need it in the future.
    2. PWAController::getCacheVersion(): this could go into a service.
    3. 🐛 [META][pwa_service_worker] Refactor 'pwa_service_worker' implementations and flaws Needs work

Broader scope stuff

🌱 Plan
Status

Active

Version

2.0

Component

Code

Created by

🇨🇦Canada ambient.impact Toronto

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024