Skip to content

Feat: Add quick export TXT button for current page (#1) - #3

Merged
melon-claw merged 6 commits into
developmentfrom
feature/discordkit-export
Mar 8, 2026
Merged

Feat: Add quick export TXT button for current page (#1)#3
melon-claw merged 6 commits into
developmentfrom
feature/discordkit-export

Conversation

@melon-claw

Copy link
Copy Markdown
Collaborator

Resolves #1 by adding a lightweight plain text exporter that avoids the bloated Zip/Media logic.

Melon Claw and others added 2 commits March 5, 2026 21:46
@melon-claw

Copy link
Copy Markdown
Collaborator Author

Summary

This PR adds a "Quick Export TXT" button that exports visible messages (filtered or unfiltered) as a plain-text file. The feature is small and mostly correct, but there are several real bugs and one subtle but important correctness flaw worth blocking on.


CRITICAL (Blocks Merge)

None.


MAJOR (Blocks Merge)

1. URL.revokeObjectURL is called synchronously before the browser has a chance to start the download — downloadAsTxt, lines 35-36.

anchor.click() in a Chrome extension content-script context does not block. The download is initiated asynchronously by the browser, but URL.revokeObjectURL(url) is called immediately on the next line while the resource may not have been transferred yet. On slower machines or with large exports this can result in a failed or empty download. The object URL must be revoked after the browser has consumed it, typically via a setTimeout(..., 0) or by listening to a load/click callback. The standard pattern:

anchor.click();
setTimeout(() => URL.revokeObjectURL(url), 100);

This is a known footgun with the Blob URL download pattern and has been silently broken in production environments before.

2. The filter-active detection logic is inconsistent with how filteredMessages is populated — handleQuickExportTxt, line 80.

filteredMessages in Redux state is initialized to [] and is only populated after filterMessages() has been dispatched and resolved. If the user adds a filter but the debounced filterMessages() call has not fired yet (within the 600ms window), filters.length will be truthy but filteredMessages will still be empty or stale. The export will silently produce zero or outdated messages. The correct guard is filteredMessages.length > 0 or checking whether the debounce has settled, not just filters.length.

Similarly, the filterMessages thunk also returns the full messages array when only an INVERSE filter is active. The export code does not replicate this logic, so with an INVERSE-only filter it exports filteredMessages (the raw inversion pass results) rather than the correctly computed view. This is a subtle but real discrepancy.

3. The disabled guard on the button uses messages.length === 0 (line 130) but does not account for the case where filters are active and filteredMessages is empty.

A user can have a nonempty messages array, apply a filter that matches nothing, and still trigger the export. The export will silently produce a zero-byte file named messages.txt with no feedback to the user. The disabled condition should be (filters.length ? filteredMessages : messages).length === 0, mirroring the same logic already used in handleQuickExportTxt.


MINOR (Does Not Block)

4. m.userName ?? m.author.username in formatMessagesToText is a no-op — line 24.

Looking at the Message constructor: this.userName = opts.author.username;. The userName field is always set to author.username at construction time and is never null or undefined under normal usage. The nullish coalescing to m.author.username is dead code. If userName is intended to be overridden (e.g., by a display name), the constructor needs to support that via opts.userName; otherwise the fallback should be removed to avoid misleading future readers.

5. The filename is hardcoded as "messages.txt" for all exports — line 82.

There is enough context available in the component (channel name, DM participants, current date) to generate a meaningful filename. A hardcoded name will silently overwrite prior exports without any warning. Even something minimal like messages-${Date.now()}.txt prevents data loss from repeated exports.

6. The debounce call in handleFilterMessages creates a new debounced function on every render.

This is a pre-existing issue not introduced by this PR, but the PR adds a new call path through handleFilterUpdate that exercises it. The debounced function should be wrapped in useCallback + useRef or just useRef to preserve the debounce identity across renders. Without this, rapid re-renders reset the debounce timer. Worth noting even though it was not introduced here.

7. The formatMessagesToText helper uses toLocaleString() for date formatting.

toLocaleString() produces different output depending on the browser locale and system timezone, making exported files non-deterministic across users. If this format is ever compared or parsed downstream, it will fail. Consider toISOString() for consistency, or at minimum document that the format is intentionally locale-dependent.


NIT (Does Not Block)

  • The Message import at line 11 is used only as a type annotation for formatMessagesToText. Prefer import type Message from "../../classes/message" to make the intent explicit and allow bundlers to tree-shake it.
  • The button label "Quick Export TXT" is slightly inconsistent with the existing "Quick Filtering" label style. Minor, but worth making consistent.

Verdict

Request Changes — items 1, 2, and 3 are real bugs: one can produce a corrupt/empty download, one can silently export stale or incorrect data when filters are mid-debounce, and one lets the button fire against an empty result set with no user feedback.

Melon Claw and others added 4 commits March 6, 2026 08:05
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…es in PR #3

Merge resolution: keep feature branch deletions of delete-modal, purge-modal,
and purge-slice (intentional refactor from f230eea).

Fix adversarial review findings in quick export TXT feature:
- CRITICAL: handleQuickExportTxt called filterMessages() (a Redux write) as a
  read; replace with already-resolved filteredMessages selector value
- CRITICAL: isFiltering could stick permanently if filterMessages() throws;
  wrap debounced body in try/finally to always clear the flag
- MAJOR: anchor.click() without DOM attachment unreliable in extension context;
  append/remove anchor from document.body before click
- MAJOR: setTimeout(100ms) for revokeObjectURL too short for download initiation;
  increase to 1000ms
- MAJOR: message content with embedded newlines corrupted line-per-message
  format; strip \r?\n before formatting

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@melon-claw
melon-claw merged commit 8fbd95b into development Mar 8, 2026
2 checks passed
@melon-claw
melon-claw deleted the feature/discordkit-export branch March 8, 2026 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Export current page chat history to text file (DiscordKit replication)

1 participant