Skip to content
This repository has been archived by the owner on Sep 26, 2024. It is now read-only.

Add utility for universal text replacement #53

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion apps/mocksi-lite/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@
],
"web_accessible_resources": [
{
"resources": ["./content/content.tsx"],
"resources": [
"./content/content.tsx",
"/*.map"
],
"matches": ["<all_urls>"]
}
],
Expand Down
90 changes: 90 additions & 0 deletions apps/mocksi-lite/universalReplace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@

class UniversalReplace {
observer: MutationObserver | undefined;
patterns: { pattern: RegExp, replace: string }[] = [];

constructor() {
}
Comment on lines +6 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
constructor() {
}
Tools
Biome

[error] 6-7: This constructor is unnecessary. (lint/complexity/noUselessConstructor)

Unsafe fix: Remove the unnecessary constructor.


addPattern(pattern: string | RegExp, replace: string) {
this.patterns.push({ pattern: toRegExpPattern(pattern), replace });

if (!this.observer) {
this.createObserver();
}
}

removePattern(pattern: string | RegExp) {
const pattern_ = toRegExpPattern(pattern);
let idx = this.patterns.findIndex(p => p.pattern.source === pattern_.source);
if (idx >= 0) this.patterns.splice(idx, 1);

if (this.patterns.length === 0) {
this.observer?.disconnect();
this.observer = undefined;
}
}

createObserver() {
const this_ = this;
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());
}
}
}
}
}
});
Comment on lines +30 to +55
Copy link

@coderabbitai coderabbitai bot Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, also changing that you won't need to declare const this_ = this;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.


this.observer.observe(document, { childList: true, subtree: true });
}

matchReplacePattern(text: string): { pattern: RegExp, replace: string } | null {
for (let pattern of this.patterns) {
if (pattern.pattern.test(text)) {
return { pattern: pattern.pattern, replace: pattern.replace }
}
}

return null
}
}

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;
}
}
Comment on lines +71 to +79
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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.

}

const toRegExpPattern = (pattern: string | RegExp) => {
if (typeof pattern === "string") {
return new RegExp('\\b' + pattern + '\\b', "ig");
}

return pattern;
}

export default new UniversalReplace();
Loading