Add utility for universal text replacement #53
Conversation
WalkthroughWalkthroughThe updates introduce significant dynamic content replacement capabilities to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DOM
participant UniversalReplace
participant MutationObserver
User->>DOM: Loads the Web Page
DOM->>UniversalReplace: Initializes Replacement Engine
UniversalReplace->>MutationObserver: Start Observing DOM Changes
DOM->>MutationObserver: Detects DOM Mutation
MutationObserver->>UniversalReplace: Reports Mutation
UniversalReplace->>DOM: Applies Pattern Replacements
DOM->>User: Displays Updated Content
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/mocksi-lite/manifest.json (1 hunks)
- apps/mocksi-lite/universalReplace.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/mocksi-lite/manifest.json
Additional context used
Biome
apps/mocksi-lite/universalReplace.ts
[error] 6-7: This constructor is unnecessary. (lint/complexity/noUselessConstructor)
Unsafe fix: Remove the unnecessary constructor.
[error] 30-55: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 76-78: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Additional comments not posted (5)
apps/mocksi-lite/universalReplace.ts (5)
9-15: Well-implemented method for adding patterns. Consider adding comments to explain the logic, especially how patterns are transformed and stored.
17-26: Good implementation of pattern removal and efficient management of the observer based on the patterns array length.
60-68: Efficient implementation of pattern matching. Ensure comprehensive unit tests cover various text inputs and pattern complexities.
82-88: Well-implemented utility function for converting patterns to regular expressions. Ensure edge cases and special characters in patterns are handled correctly.
90-90: Appropriate use of default export for a utility class that is likely intended to be used as a singleton.
| constructor() { | ||
| } |
There was a problem hiding this comment.
Remove the unnecessary constructor to simplify the class definition.
- constructor() {
- }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| constructor() { | |
| } |
Tools
Biome
[error] 6-7: This constructor is unnecessary. (lint/complexity/noUselessConstructor)
Unsafe fix: Remove the unnecessary constructor.
| this.observer = new MutationObserver(function (mutations) { | ||
| for (let mutation of mutations) { | ||
| if (mutation.addedNodes != null && mutation.addedNodes.length > 0) { | ||
| for (let node of mutation.addedNodes) { | ||
| if (node instanceof Text && | ||
| node.parentElement && | ||
| !(node.parentElement instanceof HTMLScriptElement) && | ||
| !(node.parentElement instanceof HTMLStyleElement) && | ||
| node.textContent !== null && | ||
| !(/^\s*$/.test(node.textContent))) | ||
| { | ||
| const replace = this_.matchReplacePattern(node.textContent); | ||
| if (replace) { | ||
| const treeWalker = document.createTreeWalker(node, NodeFilter.SHOW_TEXT); | ||
| let textNode: Node | ||
| do { | ||
| textNode = treeWalker.currentNode; | ||
| if (textNode.nodeValue === null) continue; | ||
| textNode.nodeValue = textNode.nodeValue.replace(replace.pattern, replaceFirstLetterCase(replace.replace)); | ||
| } while (treeWalker.nextNode()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Refactor the function expression to an arrow function to enhance readability and maintain consistency with modern JavaScript practices.
- this.observer = new MutationObserver(function (mutations) {
+ this.observer = new MutationObserver((mutations) => {Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 30-55: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
There was a problem hiding this comment.
Yeah, also changing that you won't need to declare const this_ = this;
There was a problem hiding this comment.
@NicoMorenoSirius You're absolutely right! By converting the function to an arrow function, this automatically refers to the enclosing context, eliminating the need for const this_ = this;. Here’s how the refactored code would look:
createObserver() {
this.observer = new MutationObserver((mutations) => {
for (let mutation of mutations) {
if (mutation.addedNodes != null && mutation.addedNodes.length > 0) {
for (let node of mutation.addedNodes) {
if (node instanceof Text &&
node.parentElement &&
!(node.parentElement instanceof HTMLScriptElement) &&
!(node.parentElement instanceof HTMLStyleElement) &&
node.textContent !== null &&
!(/^\s*$/.test(node.textContent)))
{
const replace = this.matchReplacePattern(node.textContent);
if (replace) {
const treeWalker = document.createTreeWalker(node, NodeFilter.SHOW_TEXT);
let textNode;
do {
textNode = treeWalker.currentNode;
if (textNode.nodeValue === null) continue;
textNode.nodeValue = textNode.nodeValue.replace(replace.pattern, replaceFirstLetterCase(replace.replace));
} while (treeWalker.nextNode());
}
}
}
}
});
}This change simplifies the code and enhances its readability. If there's anything else you'd like to adjust or discuss, feel free to let me know!
Learnings added
User: NicoMorenoSirius
PR: Mocksi/HARlighter#53
File: apps/mocksi-lite/universalReplace.ts:30-55
Timestamp: 2024-06-17T19:21:12.952Z
Learning: Arrow functions capture the `this` value of the enclosing context at the time they are created, making the use of `const this_ = this;` redundant when refactoring from a traditional function.
Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure review instructions.
| const replaceFirstLetterCase = (value: string) => { | ||
| return (match: string) => { | ||
| // Check if the first letter in the match is uppercase | ||
| if (match[0] === match[0].toUpperCase()) { | ||
| return value.charAt(0).toUpperCase() + value.slice(1); | ||
| } else { | ||
| return value; | ||
| } | ||
| } |
There was a problem hiding this comment.
Simplify the function by removing the unnecessary else clause.
- } else {
- return value;
- }
+ return value;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const replaceFirstLetterCase = (value: string) => { | |
| return (match: string) => { | |
| // Check if the first letter in the match is uppercase | |
| if (match[0] === match[0].toUpperCase()) { | |
| return value.charAt(0).toUpperCase() + value.slice(1); | |
| } else { | |
| return value; | |
| } | |
| } | |
| const replaceFirstLetterCase = (value: string) => { | |
| return (match: string) => { | |
| // Check if the first letter in the match is uppercase | |
| if (match[0] === match[0].toUpperCase()) { | |
| return value.charAt(0).toUpperCase() + value.slice(1); | |
| } | |
| return value; | |
| } | |
| } |
Tools
Biome
[error] 76-78: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
|
Closing because this branch is inside this PR: |
Adds a utility to do universal text replacement using a MutationObserver. It automatically adds and removes the observer as patterns are added. To use it:
import UniversalReplace from "../universalReplace";
...
UniversalReplace.addPattern('the', 'teh');
You can also add regular expressions as patterns, but note that the equivalent of 'the' is /\bthe\b/ig
Summary by CodeRabbit
New Features
Improvements
mocksi-liteapp to include.mapfiles as web-accessible resources, improving debugging and development experience.