refactor(Webpack): Centralize and refactor Webpack patching

[Nuckyz]

Centralizes Webpack patching around a single thing: Patching the module factories object of the WebpackRequire (wreq.m). This wraps the modules object with a proxy to intercept the addition of new modules, and allow us to decide what to do with them.

A new internal setting was added, eagerPatches, and it decides what to do with the newly added modules. With it enabled, we patch the factories as soon as they are set, and with it disabled we only patch them once they are accessed for the first time.

For correctly handling patching, because of multiple WebpackInstances, and to allow for patching factories only when they are accessed, our proxy defines getters and setters for each module factory added, instead of doing a simple Reflect.set on the module factories object.

Obtaining the main WebpackRequire instance is now also handled by the same way we patch wreq.m, as inside of the wreq.O setter we define logic to check if this wreq instance is the main one, and initialize our references if it is.

Additionally, this PR does a lot of clean up related to the main Webpack patching things. This includes changes to the reporter code (which was also modified to fit the removal of the beforeListeners API), variable names, stringifying of modules and QOL changes for better DevTools analyzing.

I also took the opportunity to correctly document all of the WebpackRequire instance, including full typings of it and docs of its internal methods (all of the ones present in the main Discord WebpackRequire). Our code utilizes the newer and correct typings now.

The patchedFactory wrapper is more future proof now and has a fallback logic if we fail to acquire the main WebpackRequire instance, for whatever the reason is. The first time it is called, it will attempt to use the value of the require argument of the factory as our internal reference to the WebpackRequire.

~~and yes, this completely removes the refactor I did 2 months ago~~

[Nuckyz]

@CodiumAI-Agent /review

[Nuckyz]

@CodiumAI-Agent /improve

[QodoAI-Agent]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add null checks for wreq and wreq.m to prevent runtime errors Consider checking if wreq and wreq.m are not null or undefined before iterating over wreq.m. This will prevent potential runtime errors if wreq or wreq.m are not properly initialized. scripts/generateReport.ts [470-471] -for (const id in wreq!.m) { - wreq!.m[id]; +if (wreq && wreq.m) { + for (const id in wreq.m) { + wreq.m[id]; + } } Suggestion importance[1-10]: 10 Why: Adding null checks for wreq and wreq.m is crucial to prevent potential runtime errors if these objects are not properly initialized. This suggestion addresses a possible bug and enhances the robustness of the code.	10
	Add a check to ensure the target value is not undefined before getting its property descriptor The condition in handler.getOwnPropertyDescriptor should check if targetkGET is not undefined before attempting to get its property descriptor to avoid potential runtime errors. src/utils/lazy.ts [70] -const descriptor = Reflect.getOwnPropertyDescriptor(target[kGET](), p); +const value = target[kGET](); +const descriptor = value ? Reflect.getOwnPropertyDescriptor(value, p) : undefined; Suggestion importance[1-10]: 9 Why: Adding a check to ensure that target[kGET]() is not undefined before attempting to get its property descriptor is important to prevent potential runtime errors. This suggestion addresses a possible bug and improves code safety.	9
	Add error handling for cases where patchFactory returns null or undefined Consider handling the case where patchedMod might be undefined or null after the patchFactory call, to prevent potential runtime errors. src/webpack/patchWebpack.ts [30-31] const patchedMod = patchFactory(p, mod); -Reflect.set(target, p, patchedMod); +if (patchedMod) { + Reflect.set(target, p, patchedMod); +} else { + logger.warn(`Failed to patch module with id: ${p}`); +} Suggestion importance[1-10]: 9 Why: This suggestion adds important error handling to prevent potential runtime errors if patchFactory returns null or undefined, enhancing the robustness of the code.	9
Enhancement	Use a functional approach to combine arrays for better clarity and immutability Instead of directly modifying the keys array within the loop, consider using a more functional approach with Array.concat or spread syntax to enhance code clarity and immutability. src/utils/lazy.ts [60-61] -for (const key of UNCONFIGURABLE_PROPERTIES) { - if (!keys.includes(key)) keys.push(key); -} +const allKeys = [...keys, ...UNCONFIGURABLE_PROPERTIES.filter(key => !keys.includes(key))]; Suggestion importance[1-10]: 7 Why: Using a functional approach with Array.concat or spread syntax can improve code clarity and maintainability by avoiding direct modification of the keys array. This is a good enhancement but not critical.	7
	Improve readability and adopt functional programming practices by using array methods Replace the for loop with Array.prototype.filter and Array.prototype.concat for better readability and functional programming practices. src/webpack/patchWebpack.ts [37-40] const keys = Reflect.ownKeys(target); -for (const key of UNCONFIGURABLE_PROPERTIES) { - if (!keys.includes(key)) keys.push(key); -} +const additionalKeys = UNCONFIGURABLE_PROPERTIES.filter(key => !keys.includes(key)); +const finalKeys = keys.concat(additionalKeys); Suggestion importance[1-10]: 6 Why: The suggestion improves readability and adopts functional programming practices, but the performance impact of this change is minimal, making it a minor enhancement.	6
Maintainability	Improve type safety by replacing any with more specific types or interfaces Replace the use of any type with more specific types or interfaces to improve type safety and maintainability of the code. src/webpack/patchWebpack.ts [20-42] -const modulesProxyhandler: ProxyHandler<any> = { +interface Module { + [key: string]: any; // Define more specific types if possible +} + +const modulesProxyhandler: ProxyHandler<Module> = { ...Object.fromEntries(Object.getOwnPropertyNames(Reflect).map(propName => - [propName, (target: any, ...args: any[]) => Reflect[propName](target, ...args)] + [propName, (target: Module, ...args: any[]) => Reflect[propName](target, ...args)] )), - get: (target, p: string) => { + get: (target: Module, p: string) => { const mod = Reflect.get(target, p); ... }, - set: (target, p, newValue) => Reflect.set(target, p, newValue), - ownKeys: target => { + set: (target: Module, p: string, newValue: any) => Reflect.set(target, p, newValue), + ownKeys: (target: Module) => { const keys = Reflect.ownKeys(target); ... } }; Suggestion importance[1-10]: 7 Why: The suggestion improves type safety and maintainability by replacing any with a more specific type. However, the suggested Module interface still uses any for its properties, which only partially addresses the issue.	7
Best practice	Use an enum for unconfigurable properties to enhance type safety and maintainability Consider using a TypeScript enum for UNCONFIGURABLE_PROPERTIES instead of an array for better type safety and easier refactoring in future. src/utils/misc.tsx [104] -export const UNCONFIGURABLE_PROPERTIES = ["arguments", "caller", "prototype"]; +export enum UnconfigurableProperties { + Arguments = "arguments", + Caller = "caller", + Prototype = "prototype" +} Suggestion importance[1-10]: 6 Why: Using a TypeScript enum for UNCONFIGURABLE_PROPERTIES can enhance type safety and make future refactoring easier. This is a best practice suggestion that improves code maintainability but is not critical.	6

[Nuckyz]

@CodiumAI-Agent /improve

[QodoAI-Agent]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for the promise to prevent unhandled promise rejections Consider adding error handling for the promise returned by searchAndLoadLazyChunks. Currently, if the promise rejects, it could lead to unhandled promise rejections, which are deprecated and can terminate the Node.js process. scripts/generateReport.ts [358] -searchAndLoadLazyChunks(String(factory)).then(() => isResolved = true); +searchAndLoadLazyChunks(String(factory)).then(() => isResolved = true).catch(err => console.error("Failed to load lazy chunks:", err)); Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential bug that could cause unhandled promise rejections, which are deprecated and can terminate the Node.js process. Adding error handling improves the robustness of the code.	9
	Add error handling around dynamic module loading to prevent runtime crashes The wreq function call should include error handling to manage cases where the module loading fails, especially since this is a dynamic operation that could lead to runtime errors. scripts/generateReport.ts [444] -if (wreq.m[entryPoint]) wreq(entryPoint); +if (wreq.m[entryPoint]) { + try { + wreq(entryPoint); + } catch (error) { + console.error("Failed to require module:", entryPoint, error); + } +} Suggestion importance[1-10]: 9 Why: Adding error handling around the dynamic module loading is crucial to prevent runtime crashes. This suggestion enhances the stability and reliability of the code.	9
	Add a check to ensure webpackRequire.c exists before modifying its properties Consider checking the existence of webpackRequire.c before assigning properties to it in _initWebpack. This will prevent potential runtime errors if webpackRequire.c is undefined. src/webpack/webpack.ts [81-86] -Reflect.defineProperty(webpackRequire.c, Symbol.toStringTag, { - value: "ModuleCache", - configurable: true, - writable: true, - enumerable: false -}); +if (webpackRequire.c) { + Reflect.defineProperty(webpackRequire.c, Symbol.toStringTag, { + value: "ModuleCache", + configurable: true, + writable: true, + enumerable: false + }); +} else { + logger.error("webpackRequire.c is undefined"); +} Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential runtime error by ensuring that webpackRequire.c exists before attempting to modify its properties. This is a crucial check that enhances the robustness of the code.	9
	Handle potential null or undefined values returned by getProperty The maybeGetError function should handle the case where getProperty("message") returns null or undefined, which can happen if the property does not exist on the handle object. scripts/generateReport.ts [45-46] return (handle as JSHandle<Error>)?.getProperty("message") - .then(m => m.jsonValue()); + .then(m => m ? m.jsonValue() : "No error message found"); Suggestion importance[1-10]: 8 Why: This suggestion improves error handling by accounting for cases where getProperty("message") might return null or undefined. This ensures that the function handles such cases gracefully, preventing potential runtime errors.	8
Enhancement	Remove the redundant async keyword from the function declaration The async keyword is redundant in the maybeGetError function since it does not use await within its body. Removing it can avoid confusion about the function's behavior. scripts/generateReport.ts [44-47] -async function maybeGetError(handle: JSHandle) { +function maybeGetError(handle: JSHandle) { return (handle as JSHandle<Error>)?.getProperty("message") .then(m => m.jsonValue()); } Suggestion importance[1-10]: 7 Why: Removing the redundant async keyword clarifies the function's behavior and improves code readability. However, this is a minor enhancement compared to the other suggestions.	7
Maintainability	Improve type safety by using a more specific interface instead of a generic any type Replace the direct type casting of ModuleExports[] with a more type-safe approach using generics or interfaces to ensure type safety and maintainability. src/webpack/webpack.ts [138] -const ret: ModuleExports[] = []; +interface IModuleExports { + // Define the expected properties of ModuleExports here +} +const ret: IModuleExports[] = []; Suggestion importance[1-10]: 7 Why: This suggestion improves type safety and maintainability by replacing the generic any type with a more specific interface. While beneficial, it requires defining the expected properties of ModuleExports, which may not be straightforward.	7
	Improve variable naming for better code readability Use a more descriptive variable name than str for the string representation of module factories in findModuleId. This enhances code readability and maintainability. src/webpack/webpack.ts [227] -const str = String(wreq.m[id]); +const moduleAsString = String(wreq.m[id]); Suggestion importance[1-10]: 6 Why: This suggestion enhances code readability by using a more descriptive variable name. While it improves maintainability, it is a minor change and does not address any critical issues.	6

[Nuckyz]

I updated the PR description to explain what changed

[Nuckyz]

Superseded by https://github.com/Vendicated/Vencord/pull/2409