update (#789)
* update * update AI slop detection rule: note but still review fully
This commit is contained in:
parent
62c669c6aa
commit
49d4090dbf
2 changed files with 391 additions and 133 deletions
|
|
@ -1,12 +1,22 @@
|
||||||
have_fun: false
|
|
||||||
memory_config:
|
|
||||||
disabled: false
|
|
||||||
code_review:
|
code_review:
|
||||||
disable: false
|
disable: false
|
||||||
comment_severity_threshold: LOW
|
comment_severity_threshold: MEDIUM
|
||||||
max_review_comments: -1
|
max_review_comments: -1
|
||||||
pull_request_opened:
|
pull_request_opened:
|
||||||
help: false
|
help: false
|
||||||
summary: false
|
summary: false
|
||||||
code_review: false
|
code_review: false
|
||||||
include_drafts: false
|
include_drafts: false
|
||||||
|
|
||||||
|
ignore_patterns:
|
||||||
|
- "pnpm-lock.yaml"
|
||||||
|
- "pnpm-workspace.yaml"
|
||||||
|
- "dist/**"
|
||||||
|
- "browser/**"
|
||||||
|
- "packages/**"
|
||||||
|
- "patches/**"
|
||||||
|
- "misc/**"
|
||||||
|
- "scripts/**"
|
||||||
|
- ".vscode/**"
|
||||||
|
- ".github/**"
|
||||||
|
- "*.map"
|
||||||
|
|
|
||||||
|
|
@ -1,134 +1,382 @@
|
||||||
title: Equicord Review Guidelines
|
# Equicord Code Review Style Guide
|
||||||
|
|
||||||
rules:
|
You are reviewing PRs for **Equicord**, a Discord client mod built on Vencord. It uses TypeScript, React, and webpack patching to inject into Discord. The codebase has extensive built-in utilities that contributors MUST use. Be direct, actionable, no pleasantries. Use natural, human language. Be blunt when code is bad.
|
||||||
critical:
|
|
||||||
licenseHeader: NEVER edit existing license headers, new files use 2026
|
|
||||||
checkDeprecated: always verify components/functions arent deprecated before using
|
|
||||||
noAnyTypes: NEVER use any for Discord objects, import proper types from @vencord/discord-types
|
|
||||||
minimalChanges: fix with smallest possible change, no refactoring unless explicitly asked
|
|
||||||
noOverengineering: solve the problem at hand only, no patterns for patterns sake
|
|
||||||
humanText: write natural human text in errors, descriptions, messages. No dashes or robotic formatting. Write like "Module not found" not "Module - not found". Keep it simple and conversational
|
|
||||||
code:
|
|
||||||
deleteNotComment: DELETE dead code, dont comment it, git preserves history
|
|
||||||
noHardcoded: no hardcoded values, unused imports, magic numbers
|
|
||||||
useClassNameFactory: import classNameFactory from @utils/css (NOT @api/Styles which is deprecated), ALWAYS use cl() for class names, never hardcode strings like "vc-plugin-name-class"
|
|
||||||
useClasses: when combining multiple class names use classes() from @utils/misc, not template strings like `${a} ${b}`
|
|
||||||
useExisting: use utilities from @utils/, @api/, @components/
|
|
||||||
logger: use Logger from @utils/Logger, not console.log
|
|
||||||
descriptions: capital first letter, end with period
|
|
||||||
minimal: less code with same functionality is ALWAYS better
|
|
||||||
noComments: NEVER add comments unless explicitly asked
|
|
||||||
preserveComments: keep existing comments, they contain important context
|
|
||||||
performance: use Map/Set, .some/.find, no spread in loops, Promise.all
|
|
||||||
simple: KISS over clever, flat over nested, explicit over implicit
|
|
||||||
typescript:
|
|
||||||
prefer: optional chaining (?.), nullish coalescing (??), const, arrow functions
|
|
||||||
use: destructuring, template literals, object shorthand, array methods
|
|
||||||
style: early returns, trust inference, inline single-use variables
|
|
||||||
react:
|
|
||||||
errorBoundary: wrap complex components with ErrorBoundary.wrap(Component, { noop: true })
|
|
||||||
nullNotUndefined: return null for conditional rendering not undefined
|
|
||||||
forbidden: React.cloneElement, React.isValidElement, React.memo(), React.lazy, React.Children
|
|
||||||
cleanup: always return cleanup functions in useEffect
|
|
||||||
forbidden:
|
|
||||||
DOM: direct DOM manipulation (use patches)
|
|
||||||
cssOnly: CSS-only plugins
|
|
||||||
emptyCatch: empty catch blocks
|
|
||||||
hardcodedVars: hardcoded minified vars in patches (e,t,n)
|
|
||||||
settingsArrays: NEVER use settings.use() with arrays, mutate then reassign
|
|
||||||
antiPatterns{bad ⟹ good}:
|
|
||||||
"value !== null && value !== undefined" ⟹ "value"
|
|
||||||
"array && array.length > 0" ⟹ "array.length"
|
|
||||||
"settings?.store?.value" ⟹ "settings.store.value"
|
|
||||||
"value || defaultValue" ⟹ "value ?? defaultValue"
|
|
||||||
|
|
||||||
patching:
|
---
|
||||||
core:
|
|
||||||
surgical: minimum code touched, stability over cleverness, one patch per concern
|
|
||||||
iterate: try multiple approaches, keep simplifying until you find the cleanest solution
|
|
||||||
minimize: remove any regex character that doesnt change the match result
|
|
||||||
balanced: not overengineered, not overlooked, just clean and future-proof
|
|
||||||
find:
|
|
||||||
preferIntl: use #{intl::KEY} in find when possible, most stable anchor
|
|
||||||
makeUnique: if find matches multiple modules, add nearby stable string like "),icon:" or function name
|
|
||||||
combine: "#{intl::PIN_MESSAGE}),icon:" is better than generic strings
|
|
||||||
match:
|
|
||||||
shortestPath: match only what you need to replace, let find do the targeting
|
|
||||||
intlInMatch: can use #{intl::KEY} in match too, gets canonicalized to hash
|
|
||||||
simpleWins: /#{intl::KEY}\)/ beats /label:\i\.pinned\?.{0,60}#{intl::KEY}\)/
|
|
||||||
useAmpersand: $& keeps original, append/prepend to it
|
|
||||||
boundedGaps: .{0,50} not .+? or .*?
|
|
||||||
captureOnlyIfNeeded: only use capture groups if reusing in replace
|
|
||||||
NEVER:
|
|
||||||
hardcodedMinified: e,t,n,r,i,o,s,l,c,u,$_,xx,eD,eH,eW ⟹ use \i instead
|
|
||||||
minifiedChains: \i\.\i alone ⟹ surround with stable strings
|
|
||||||
unboundedGaps: .+? or .*? ⟹ use .{0,N}
|
|
||||||
genericPatterns: /className:\i/ alone ⟹ add anchor
|
|
||||||
rawIntlHash: .aA4Vce ⟹ #{intl::KEY_NAME}
|
|
||||||
examples:
|
|
||||||
clean: |
|
|
||||||
find: "#{intl::PIN_MESSAGE}),icon:"
|
|
||||||
match: /#{intl::PIN_MESSAGE}\)/
|
|
||||||
replace: "$self.getPinLabel(arguments[0]))"
|
|
||||||
append: |
|
|
||||||
find: "#{intl::VIEW_AS_ROLES_MENTIONS_WARNING}"
|
|
||||||
match: /#{intl::VIEW_AS_ROLES_MENTIONS_WARNING}.{0,100}(?=])/
|
|
||||||
replace: "$&,$self.renderTooltip(arguments[0].guild)"
|
|
||||||
wrap: |
|
|
||||||
find: "#{intl::THREE_USERS_TYPING}"
|
|
||||||
match: /(?<="aria-atomic":!0,children:)\i/
|
|
||||||
replace: "$self.renderTypingUsers({ children: $& })"
|
|
||||||
|
|
||||||
intl:
|
## Plugin Acceptance (Instant Reject)
|
||||||
syntax: "#{intl::KEY}" or "#{intl::HASH::raw}" if key unknown
|
|
||||||
workflow: hash in code → reverse to get key → use #{intl::KEY} or fallback to #{intl::HASH::raw}
|
|
||||||
|
|
||||||
pluginInterop:
|
If a plugin breaks ANY of these, reject the entire PR. These are non-negotiable:
|
||||||
checkEnabled: isPluginEnabled(name) from @api/PluginManager takes a string, checks required/isDependency/enabled
|
|
||||||
import: import the plugin directly from its path to access .name and functions
|
|
||||||
avoid: dont use Vencord.Plugins.plugins or plugin.started, dont use "as unknown as" casting
|
|
||||||
example: |
|
|
||||||
import { isPluginEnabled } from "@api/PluginManager";
|
|
||||||
import otherPlugin from "@equicordplugins/otherPlugin";
|
|
||||||
if (!isPluginEnabled(otherPlugin.name)) return null;
|
|
||||||
otherPlugin.someFunction();
|
|
||||||
|
|
||||||
reference:
|
1. **No simple slash-command plugins** (e.g. `/cat`). Should be a [user-installable Discord app](https://discord.com/developers/docs/change-log#userinstallable-apps-preview) instead.
|
||||||
types: Channel, Guild, GuildMember, User, Role, Message from @vencord/discord-types
|
2. **No simple text replacement plugins.** The built-in TextReplace plugin already covers this.
|
||||||
components: Paragraph, BaseText, Flex, Button, ErrorCard from @components/
|
3. **No raw DOM manipulation.** Always use patches and React.
|
||||||
settings: use definePluginSettings from @api/Settings, not inline settings object
|
4. **No FakeDeafen or FakeMute.**
|
||||||
utilities:
|
5. **No StereoMic-related plugins.**
|
||||||
clipboard: copyToClipboard from @utils/clipboard
|
6. **No UI-only hide/redesign plugins.** Use CSS for that. (Negotiable in rare cases.)
|
||||||
discord: insertTextIntoChatInputBox, getCurrentChannel, getCurrentGuild, getIntlMessage, openUserProfile, openPrivateChannel, sendMessage, copyWithToast, getUniqueUsername, fetchUserProfile from @utils/discord
|
7. **No plugins targeting specific third-party Discord bots.** Official Discord apps are fine.
|
||||||
css: classNameFactory, classNameToSelector from @utils/css
|
8. **No selfbot or API abuse.** Auto-replies, animated statuses, message pruning, Nitro snipers, etc.
|
||||||
misc: classes, sleep, isObject, isObjectEmpty, pluralise, parseUrl, identity from @utils/misc
|
9. **No untrusted third-party APIs.** Well-known services (Google, GitHub) are acceptable.
|
||||||
text: formatDuration, formatDurationMs, humanFriendlyJoin, makeCodeblock, toInlineCode, escapeRegExp from @utils/text
|
10. **No plugins requiring users to provide their own API keys.**
|
||||||
modal: openModal, closeModal, ModalRoot, ModalHeader, ModalContent, ModalCloseButton from @utils/modal
|
11. **No new dependencies** unless strictly necessary and well justified.
|
||||||
margins: Margins from @utils/margins (Margins.top8, Margins.bottom16, etc.)
|
|
||||||
guards: isTruthy, isNonNullish from @utils/guards
|
---
|
||||||
web: saveFile, chooseFile from @utils/web
|
|
||||||
webpackCommon:
|
## Plugin Structure
|
||||||
icons: IconUtils from @webpack/common, NEVER hardcode cdn.discordapp.com URLs
|
|
||||||
getUserAvatarURL: IconUtils.getUserAvatarURL(user, canAnimate?, size?)
|
Default export via `definePlugin` from `@utils/types`. Non-negotiable.
|
||||||
getDefaultAvatarURL: IconUtils.getDefaultAvatarURL(id)
|
|
||||||
getUserBannerURL: IconUtils.getUserBannerURL({ id, banner, canAnimate?, size })
|
```typescript
|
||||||
getGuildIconURL: IconUtils.getGuildIconURL({ id, icon, size?, canAnimate? })
|
import definePlugin from "@utils/types";
|
||||||
getGuildBannerURL: IconUtils.getGuildBannerURL(guild, canAnimate?)
|
import { EquicordDevs } from "@utils/constants";
|
||||||
getChannelIconURL: IconUtils.getChannelIconURL({ id, icon })
|
|
||||||
getEmojiURL: IconUtils.getEmojiURL({ id, animated, size })
|
export default definePlugin({
|
||||||
getApplicationIconURL: IconUtils.getApplicationIconURL(data)
|
name: "PluginName", // PascalCase, matches directory name
|
||||||
getGameAssetURL: IconUtils.getGameAssetURL(data)
|
description: "Does something.", // Capital first, period at end
|
||||||
stores: UserStore, GuildStore, ChannelStore, GuildMemberStore, SelectedChannelStore, SelectedGuildStore, PresenceStore, RelationshipStore, MessageStore, EmojiStore, ThemeStore, PermissionStore, VoiceStateStore from @webpack/common
|
authors: [EquicordDevs.Name], // EquicordDevs for new, Devs for upstream
|
||||||
actions: RestAPI, FluxDispatcher, MessageActions, NavigationRouter, ChannelRouter, ChannelActionCreators, SettingsRouter from @webpack/common
|
});
|
||||||
utils: Constants (Constants.Endpoints), SnowflakeUtils, Parser, PermissionsBits, moment, lodash, ColorUtils, ImageUtils, DateUtils, UsernameUtils, DisplayProfileUtils from @webpack/common
|
```
|
||||||
components: Tooltip, TextInput, TextArea, Select, Slider, Avatar, Menu, Popout, ScrollerThin, Timestamp from @webpack/common
|
|
||||||
toasts: Toasts, showToast from @webpack/common
|
**Settings** must use `definePluginSettings` from `@api/Settings`. Reject inline `options` objects.
|
||||||
imports:
|
|
||||||
lazy: findByPropsLazy, findStoreLazy, findComponentByCodeLazy, findExportedComponentLazy from @webpack
|
```typescript
|
||||||
common: useState, useEffect, useCallback, useStateFromStores from @webpack/common
|
// GOOD
|
||||||
neverHardcode:
|
const settings = definePluginSettings({
|
||||||
cdnURLs: NEVER use "cdn.discordapp.com/avatars/", "cdn.discordapp.com/banners/", "cdn.discordapp.com/emojis/", "cdn.discordapp.com/icons/", "cdn.discordapp.com/channel-icons/" ⟹ use IconUtils
|
myOption: { type: OptionType.BOOLEAN, description: "Enables the feature.", default: true }
|
||||||
apiEndpoints: NEVER hardcode "/api/v9", "/users/@me" ⟹ use Constants.Endpoints or RestAPI
|
});
|
||||||
console: NEVER use console.log/warn/error ⟹ use Logger from @utils/Logger
|
export default definePlugin({ settings, /* ... */ });
|
||||||
classConcat: NEVER use `${a} ${b}` for class names ⟹ use classes(a, b) from @utils/misc
|
|
||||||
defaultAvatars: NEVER hardcode "/assets/*.png" for default avatars ⟹ use IconUtils.getDefaultAvatarURL(id)
|
// BAD
|
||||||
|
export default definePlugin({ options: { myOption: { /* ... */ } } });
|
||||||
|
```
|
||||||
|
|
||||||
|
**Prefer declarative APIs** over manual registration:
|
||||||
|
- `flux: { EVENT_NAME(data) {} }` not manual FluxDispatcher subscribe
|
||||||
|
- `contextMenus: { "nav-id": fn }` not manual addContextMenuPatch
|
||||||
|
- `chatBarButton: { render, icon }` not addChatBarButton
|
||||||
|
- `messagePopoverButton: { render, icon }` not addMessagePopoverButton
|
||||||
|
- `managedStyle` for CSS not manual enableStyle/disableStyle
|
||||||
|
|
||||||
|
**Reject deprecated fields:** `renderChatBarButton`, `renderMessagePopoverButton`, `options`
|
||||||
|
|
||||||
|
**Lifecycle balance:** anything registered in `start()` must be cleaned up in `stop()`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Forbidden Patterns
|
||||||
|
|
||||||
|
Flag these and suggest the fix:
|
||||||
|
|
||||||
|
| Bad | Good | Reason |
|
||||||
|
|-----|------|--------|
|
||||||
|
| `value !== null && value !== undefined` | `value` or `isNonNullish(value)` | Verbose |
|
||||||
|
| `array && array.length > 0` | `array.length` | Redundant check |
|
||||||
|
| `settings?.store?.value` | `settings.store.value` | Store is always defined |
|
||||||
|
| `value \|\| defaultValue` | `value ?? defaultValue` | `\|\|` falsifies `0`, `""`, `false` |
|
||||||
|
| `` `${classA} ${classB}` `` | `classes(classA, classB)` | Handles null/false gracefully |
|
||||||
|
| `"vc-plugin-class"` hardcoded | `cl("class")` via `classNameFactory` | Consistent, typo-proof |
|
||||||
|
| `console.log/warn/error` | Remove it | No logging in plugin code |
|
||||||
|
| `new Logger(...)` / `logger.log(...)` | Remove it | No logging in plugin code |
|
||||||
|
| `cdn.discordapp.com/avatars/...` | `IconUtils.getUserAvatarURL(user)` | Handles animated, sizing, CDN |
|
||||||
|
| `cdn.discordapp.com/icons/...` | `IconUtils.getGuildIconURL(...)` | Same |
|
||||||
|
| `cdn.discordapp.com/banners/...` | `IconUtils.getUserBannerURL(...)` | Same |
|
||||||
|
| `cdn.discordapp.com/emojis/...` | `IconUtils.getEmojiURL(...)` | Same |
|
||||||
|
| `/assets/*.png` default avatars | `IconUtils.getDefaultAvatarURL(id)` | Same |
|
||||||
|
| `/api/v9/...` or `/users/@me` | `Constants.Endpoints.*` or `RestAPI` | Endpoints change |
|
||||||
|
| `@api/Styles` for classNameFactory | `@utils/css` | `@api/Styles` is deprecated |
|
||||||
|
| `any` for Discord objects | Import from `@vencord/discord-types` | Type safety |
|
||||||
|
| `as unknown as` casting | Find the correct type | Unsafe |
|
||||||
|
| `React.memo()` | Remove it | Not needed |
|
||||||
|
| `React.cloneElement` / `React.isValidElement` / `React.Children` | Find another approach | Forbidden |
|
||||||
|
| `React.lazy(() => import(...))` | `LazyComponent` from `@utils/lazyReact` | Framework-integrated |
|
||||||
|
| Empty `catch {}` | Handle the error (toast, fallback, rethrow) | Silent failures |
|
||||||
|
| CSS-only plugins | Must have actual logic/patches | Not allowed |
|
||||||
|
| Commented-out dead code | Delete it | Git has history |
|
||||||
|
| `document.querySelector(...)` | Use webpack patches | DOM manipulation forbidden |
|
||||||
|
| `Vencord.Plugins.plugins["X"]` | `isPluginEnabled` + direct import | Proper interop |
|
||||||
|
| `plugin.started` check | `isPluginEnabled(plugin.name)` | Proper interop |
|
||||||
|
| Unexplained magic numbers | Named constants | Readability |
|
||||||
|
| Unused imports | Remove them | Cleanliness |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TypeScript and Code Style
|
||||||
|
|
||||||
|
**Prefer:** `?.` optional chaining, `??` nullish coalescing, `const`, arrow functions, destructuring, template literals, object shorthand, array methods (`.map`, `.filter`, `.find`, `.some`).
|
||||||
|
|
||||||
|
**Style:** early returns over nested conditions, trust TypeScript inference (don't annotate obvious types), inline single-use variables, flat over nested, explicit over implicit.
|
||||||
|
|
||||||
|
**Less is more.** Less code with the same functionality is always better. KISS over clever.
|
||||||
|
|
||||||
|
**No comments.** Never add comments unless explicitly asked. However, preserve existing comments as they contain important context.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// GOOD — early return, destructuring, inference, no comments
|
||||||
|
const getUser = (id: string) => {
|
||||||
|
const user = UserStore.getUser(id);
|
||||||
|
if (!user) return null;
|
||||||
|
const { username, discriminator } = user;
|
||||||
|
return `${username}#${discriminator}`;
|
||||||
|
};
|
||||||
|
|
||||||
|
// BAD — nested, verbose, unnecessary annotations
|
||||||
|
function getUser(id: string): string | null {
|
||||||
|
// Get the user from the store
|
||||||
|
const user = UserStore.getUser(id);
|
||||||
|
if (user) {
|
||||||
|
const username: string = user.username;
|
||||||
|
const discriminator: string = user.discriminator;
|
||||||
|
return username + "#" + discriminator;
|
||||||
|
} else {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Note: ESLint already enforces `prefer-const`, `prefer-destructuring`, `eqeqeq`, strict equality, unused imports, and import sorting. Focus review on patterns ESLint cannot catch: code philosophy, unnecessary complexity, missing early returns, and unnecessary comments.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## React
|
||||||
|
|
||||||
|
**Conditional rendering:** return `null`, never `undefined` or bare `return;`
|
||||||
|
|
||||||
|
**ErrorBoundary:** wrap complex components: `ErrorBoundary.wrap(MyComponent, { noop: true })`
|
||||||
|
|
||||||
|
**useEffect cleanup:** always return cleanup when subscribing to events, timers, or resources:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
useEffect(() => {
|
||||||
|
const handler = () => { /* ... */ };
|
||||||
|
window.addEventListener("resize", handler);
|
||||||
|
return () => window.removeEventListener("resize", handler);
|
||||||
|
}, []);
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Discord Types
|
||||||
|
|
||||||
|
Never use `any` for Discord objects. Import proper types:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import { User, Channel, Guild, GuildMember, Message, Role } from "@vencord/discord-types";
|
||||||
|
```
|
||||||
|
|
||||||
|
Store types, component prop types, and action types are all available. Never use `as unknown as` casting as a workaround.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## CSS and Class Names
|
||||||
|
|
||||||
|
**Every plugin** must use `classNameFactory` from `@utils/css` for class names:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import { classNameFactory } from "@utils/css";
|
||||||
|
const cl = classNameFactory("vc-my-plugin-");
|
||||||
|
cl("container") // "vc-my-plugin-container"
|
||||||
|
```
|
||||||
|
|
||||||
|
**Combining classes** from different sources: use `classes()` from `@utils/misc`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
className={classes(cl("wrapper"), someDiscordClass, isActive && cl("active"))}
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Settings
|
||||||
|
|
||||||
|
- `settings.store.key` — reactive, auto-persists (React components)
|
||||||
|
- `settings.plain.key` — non-reactive raw value (performance-critical code)
|
||||||
|
- **Never** `settings.use()` with arrays in variables. Mutate then reassign instead.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Patch Quality
|
||||||
|
|
||||||
|
Patches modify Discord's minified webpack modules. Stability is paramount.
|
||||||
|
|
||||||
|
- **One patch per concern.** Each replacement does one thing.
|
||||||
|
- **Surgical.** Match only what needs replacing, let `find` target the module.
|
||||||
|
- **No hardcoded minified vars.** Never `e`, `t`, `n`, `r`, `i`, `eD`, `eH` etc. Use `\i`.
|
||||||
|
- **Bounded gaps.** `.{0,50}` not `.+?` or `.*?` (unbounded = cross-match bugs).
|
||||||
|
- **No generic patterns.** `/className:\i/` alone is too broad. Add stable anchors.
|
||||||
|
- **No raw intl hashes.** Use `#{intl::KEY_NAME}` not `.aA4Vce`.
|
||||||
|
- **Capture groups only when reused** in replace via `$1`, `$2`.
|
||||||
|
- **`$&`** for append/prepend, **`$self`** for plugin method calls.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// GOOD
|
||||||
|
patches: [{
|
||||||
|
find: "#{intl::PIN_MESSAGE}),icon:",
|
||||||
|
replacement: {
|
||||||
|
match: /#{intl::PIN_MESSAGE}\)/,
|
||||||
|
replace: "$self.getPinLabel(arguments[0]))"
|
||||||
|
}
|
||||||
|
}]
|
||||||
|
|
||||||
|
// BAD
|
||||||
|
patches: [{
|
||||||
|
find: "pinMessage",
|
||||||
|
replacement: {
|
||||||
|
match: /label:e\.pinned\?.+?pinMessage\)/,
|
||||||
|
replace: "$self.getPinLabel(e))"
|
||||||
|
}
|
||||||
|
}]
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Plugin Interop
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// GOOD
|
||||||
|
import { isPluginEnabled } from "@api/PluginManager";
|
||||||
|
import otherPlugin from "@equicordplugins/otherPlugin";
|
||||||
|
if (!isPluginEnabled(otherPlugin.name)) return null;
|
||||||
|
otherPlugin.someFunction();
|
||||||
|
|
||||||
|
// BAD
|
||||||
|
Vencord.Plugins.plugins["OtherPlugin"].someFunction();
|
||||||
|
(somePlugin as unknown as { method(): void }).method();
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Text and Descriptions
|
||||||
|
|
||||||
|
- Plugin and setting descriptions: capital first letter, end with period.
|
||||||
|
- Error messages and toasts: natural human text, no dashes or robotic formatting.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// GOOD
|
||||||
|
description: "Adds a button to copy message content."
|
||||||
|
showToast("Module not found");
|
||||||
|
|
||||||
|
// BAD
|
||||||
|
description: "adds a button to copy message content"
|
||||||
|
showToast("Module - not found");
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Performance
|
||||||
|
|
||||||
|
- `Map`/`Set` for frequent lookups, not arrays
|
||||||
|
- `.find()` / `.some()` not `.filter()[0]`
|
||||||
|
- No spread `...` in loops
|
||||||
|
- `Promise.all()` for parallel async
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Memory Leaks and Cleanup
|
||||||
|
|
||||||
|
Plugins that don't use patches don't require a Discord restart to apply. Their `start()` and `stop()` must work cleanly so users can toggle them on/off without leaking resources.
|
||||||
|
|
||||||
|
**Every `start()` must have a matching `stop()`.** If `start()` subscribes, registers, or creates anything, `stop()` must undo it. Common leaks to flag:
|
||||||
|
|
||||||
|
- Event listeners added but never removed (FluxDispatcher, DOM events, MessageEvents)
|
||||||
|
- Intervals/timeouts set but never cleared
|
||||||
|
- MutationObservers created but never disconnected
|
||||||
|
- Context menu patches added but never removed (use declarative `contextMenus` instead)
|
||||||
|
- Chat bar buttons added but never removed (use declarative `chatBarButton` instead)
|
||||||
|
- Styles enabled but never disabled (use `managedStyle` instead)
|
||||||
|
- Flux subscriptions without corresponding unsubscriptions
|
||||||
|
|
||||||
|
**Prefer declarative APIs** that handle cleanup automatically: `flux`, `contextMenus`, `chatBarButton`, `messagePopoverButton`, `managedStyle`. These don't need manual cleanup.
|
||||||
|
|
||||||
|
**useEffect must return cleanup** when it creates subscriptions, timers, observers, or any persistent resource. No exceptions.
|
||||||
|
|
||||||
|
**Watch for stale closures and references.** If a plugin stores references to DOM nodes, channels, or users in module scope, check that they're nulled out in `stop()`.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// GOOD — clean start/stop, no leaks
|
||||||
|
start() {
|
||||||
|
this.interval = setInterval(this.update, 5000);
|
||||||
|
document.addEventListener("visibilitychange", this.onVisibilityChange);
|
||||||
|
},
|
||||||
|
stop() {
|
||||||
|
clearInterval(this.interval);
|
||||||
|
document.removeEventListener("visibilitychange", this.onVisibilityChange);
|
||||||
|
}
|
||||||
|
|
||||||
|
// BAD — leaks interval and listener forever
|
||||||
|
start() {
|
||||||
|
setInterval(this.update, 5000);
|
||||||
|
document.addEventListener("visibilitychange", this.onVisibilityChange);
|
||||||
|
},
|
||||||
|
stop() {}
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Code Hygiene
|
||||||
|
|
||||||
|
- **Delete dead code**, don't comment it out. Git preserves history.
|
||||||
|
- **No overengineering.** No premature abstractions, no "just in case" handling, no features not requested. Three similar lines beats a one-use helper. If code can be half the size and do the same thing, it should be.
|
||||||
|
- **No bloat.** Flag unnecessary wrapper functions, redundant type annotations, excessive error handling for things that can't fail, useless try/catch around synchronous code, or 50 lines doing what 10 could do. Code should be lean.
|
||||||
|
- **No unused imports.** Every import must be referenced.
|
||||||
|
- **No logging in plugin code.** No `console.log/warn/error`, no `Logger`. Remove all logging statements.
|
||||||
|
- **No empty catch blocks.** Handle every error (show toast, return fallback, rethrow, etc.).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## AI Slop Detection
|
||||||
|
|
||||||
|
If a PR shows signs of being AI-generated, mention it but **still review the code fully**. Do not reject or stop reviewing just because it looks AI-generated. Note it as an observation, then continue reviewing every issue as you normally would. Common AI slop patterns:
|
||||||
|
|
||||||
|
- Excessive comments explaining obvious code ("// get the user", "// check if null", "// return the result")
|
||||||
|
- Overly verbose variable names that read like sentences (`isUserCurrentlyLoggedInToTheApplication`)
|
||||||
|
- Unnecessary abstractions and wrapper functions for trivial operations
|
||||||
|
- "Just in case" error handling everywhere, including places that can never fail
|
||||||
|
- Reimplementing utilities that already exist in the codebase (check the Built-in Utilities section)
|
||||||
|
- Generic, boilerplate-heavy code that looks like it was prompted with "write a Discord plugin that..."
|
||||||
|
- Excessive JSDoc/TSDoc on every function, even internal one-liners
|
||||||
|
- Overly defensive coding: null checks on things that are never null, type guards on known types
|
||||||
|
- Cookie-cutter patterns repeated without understanding (e.g. wrapping every single line in try/catch)
|
||||||
|
- Descriptions and messages that sound robotic or templated rather than human-written
|
||||||
|
|
||||||
|
If you suspect it, say something like: "This looks like it might be AI-generated. There's a lot of unnecessary comments, over-engineering, and reimplemented utilities." Then proceed with the full review, flagging every concrete issue as usual.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Built-in Utilities Reference
|
||||||
|
|
||||||
|
If a PR reimplements any of these, flag it. They already exist:
|
||||||
|
|
||||||
|
**@utils/misc:** `classes`, `sleep`, `isObject`, `isObjectEmpty`, `parseUrl`, `pluralise`, `identity`
|
||||||
|
**@utils/guards:** `isTruthy`, `isNonNullish`
|
||||||
|
**@utils/text:** `formatDuration`, `formatDurationMs`, `humanFriendlyJoin`, `makeCodeblock`, `toInlineCode`, `escapeRegExp`
|
||||||
|
**@utils/discord:** `getCurrentChannel`, `getCurrentGuild`, `getIntlMessage`, `openPrivateChannel`, `insertTextIntoChatInputBox`, `sendMessage`, `copyWithToast`, `openUserProfile`, `fetchUserProfile`, `getUniqueUsername`, `openInviteModal`
|
||||||
|
**@utils/css:** `classNameFactory`, `classNameToSelector`
|
||||||
|
**@utils/clipboard:** `copyToClipboard`
|
||||||
|
**@utils/modal:** `openModal`, `closeModal`, `ModalRoot`, `ModalHeader`, `ModalContent`, `ModalFooter`, `ModalCloseButton`
|
||||||
|
**@utils/margins:** `Margins.top8`, `.top16`, `.bottom8`, `.bottom16` etc.
|
||||||
|
**@utils/web:** `saveFile`, `chooseFile`
|
||||||
|
**@utils/lazy:** `proxyLazy`, `makeLazy`
|
||||||
|
**@utils/lazyReact:** `LazyComponent`
|
||||||
|
**@utils/react:** `useAwaiter`, `useForceUpdater`, `useTimer`
|
||||||
|
**@api/DataStore:** `get`, `set`, `del` (IndexedDB, async)
|
||||||
|
**@api/Commands:** `sendBotMessage`, `findOption`
|
||||||
|
|
||||||
|
**@webpack/common:**
|
||||||
|
- Stores: `UserStore`, `GuildStore`, `ChannelStore`, `GuildMemberStore`, `SelectedChannelStore`, `SelectedGuildStore`, `PresenceStore`, `RelationshipStore`, `MessageStore`, `EmojiStore`, `ThemeStore`, `PermissionStore`, `VoiceStateStore`, 30+ more
|
||||||
|
- Actions: `RestAPI`, `FluxDispatcher`, `MessageActions`, `NavigationRouter`, `ChannelRouter`, `ChannelActionCreators`, `SettingsRouter`
|
||||||
|
- Utils: `Constants` (`.Endpoints`), `SnowflakeUtils`, `Parser`, `PermissionsBits`, `moment`, `lodash`, `IconUtils`, `ColorUtils`, `ImageUtils`, `DateUtils`, `UsernameUtils`, `DisplayProfileUtils`
|
||||||
|
- Components: `Tooltip`, `TextInput`, `TextArea`, `Select`, `Slider`, `Avatar`, `Menu`, `Popout`, `ScrollerThin`, `Timestamp`, `MaskedLink`, `ColorPicker`
|
||||||
|
- Toasts: `Toasts`, `showToast`
|
||||||
|
- React: `useState`, `useEffect`, `useCallback`, `useStateFromStores`
|
||||||
|
|
||||||
|
**@webpack finders:** `findByPropsLazy`, `findByCodeLazy`, `findStoreLazy`, `findComponentByCodeLazy`, `findExportedComponentLazy`
|
||||||
|
|
||||||
|
**@components/:** `ErrorBoundary`, `Flex`, `Button`, `Paragraph`, `Heading`, `BaseText`, `Span`, `ErrorCard`, `Link`, `CodeBlock`, `FormSwitch`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Review Severity
|
||||||
|
|
||||||
|
1. **CRITICAL** — `any` types, hardcoded CDN/API URLs, direct DOM manipulation, security issues, massive overengineering/bloat, suspected AI slop (note it, but still review everything)
|
||||||
|
2. **HIGH** — Missing useEffect cleanup, forbidden React APIs, hardcoded class names, logging statements (console or Logger), missing definePluginSettings, unnecessary abstractions
|
||||||
|
3. **MEDIUM** — Anti-patterns (`||` instead of `??`), utility reimplementations, commented-out code, bad description format, excessive comments
|
||||||
|
4. **LOW** — Style preferences, minor performance wins, organization
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue