Replace stylesheets

[aaronjensen]

For a while our application has had this method of replacing stylesheets that maintains the order of stylesheets as best as it can and replaces stylesheets in place if only their digest has changed. This is more sophisticated than what was there originally, but you'll notice that if you attempt to use the existing fixtures/stylesheets/left.html and right.html interactively that it doesn't work more than once. Turbo seems to hang and then refresh the page. With this method, not only does that work, but the additional fixture I put in place that ensures there's minimal to no flicker or ordering issues works as well.

I'm opening this as a draft to see if there is interest in this and/or concerns. I'm happy to clean it up if it seems like something you'd want to move forward with, which would allow us to remove our custom code from our codebase.

Thank you for the consideration.

[admsev]

@gooroodev, can you review?

[gooroodev]

Please double-check what I found in the pull request:

🐞Mistake	🤪Typo	🚨Security	🚀Performance	💪Best Practices	📖Readability	❓Others
0	0	0	0	1	1	0

Summary of Proposed Changes

• ➖ Commented out removeUnusedDynamicStylesheetElements method call in PageRenderer.
• ➕ Added removeFingerprint method to handle URL fingerprint removal.
• ➕ Added stylesheetElementIsDynamic method to check if a stylesheet element is dynamic.
• ➕ Enhanced copyNewHeadStylesheetElements method to handle dynamic stylesheet elements and maintain their order.
• ➕ Added new test fixtures for stylesheets to test the replace functionality.

Identified Issues

ID	Type	Details	Severity	Confidence
1	💪Best Practices	Commented out code should be removed if not needed. (page_renderer.js lines 80-82, 96-104, 114-118)	🟡Low	🟡Low
2	📖Readability	Console logs should be removed or replaced with proper logging mechanism. (page_renderer.js lines 112, 126, 131, 136, 141, 147, 153)	🟡Low	🟡Low

Issue 1: Commented out code should be removed if not needed

Details

Commented out code can clutter the codebase and reduce readability. If the code is not needed, it should be removed. This occurs in the following lines:

• page_renderer.js lines 80-82, 96-104, 114-118

Fix

Remove the commented out code.

// Remove the following commented out code:

// if (this.willRender) {
//   this.removeUnusedDynamicStylesheetElements()
// }

// async copyNewHeadStylesheetElements() {
//   const loadingElements = []
//
//   for (const element of this.newHeadStylesheetElements) {
//     loadingElements.push(waitForLoad(element))
//
//     document.head.appendChild(element)
//   }
//
//   await Promise.all(loadingElements)
// }

// removeUnusedDynamicStylesheetElements() {
//   for (const element of this.unusedDynamicStylesheetElements) {
//     document.head.removeChild(element)
//   }
// }

Issue 2: Console logs should be removed or replaced with proper logging mechanism

Details

Console logs are useful for debugging but should be removed or replaced with a proper logging mechanism in production code. This occurs in the following lines:

• page_renderer.js lines 112, 126, 131, 136, 141, 147, 153

Fix

Remove the console logs or replace them with a proper logging mechanism.

// Example of removing console logs:

// console.log(`Skipping ${newHref}`)
// console.log(`Updating ${oldElement.getAttribute("href")} to ${newHref}`)
// console.log(`Appending ${newHref} after`, lastStylesheetElement)
// console.log(`Removing ${element.getAttribute("href")}`)
// console.log(`Converting preload ${element.getAttribute("href")}`)

General Review

The proposed changes introduce new functionality to handle dynamic stylesheet elements more effectively, maintaining their order and removing old stylesheets when necessary. The code quality is generally good, but there are some areas for improvement:

1. Remove commented out code to keep the codebase clean.
2. Replace console logs with a proper logging mechanism or remove them if not needed.

Overall, the changes are well-structured and improve the handling of dynamic stylesheets.

--
I only arrive when I am mentioned and asked to review the pull request.
React or reply to let me know your feedback!

[aaronjensen]

Nothing about this is ready to review other than the technique.