Migrate to TypeScript with comprehensive test suite and modern tooling#1
Migrate to TypeScript with comprehensive test suite and modern tooling#1manuelgruber merged 25 commits intomainfrom
Conversation
The extension was failing silently on sites using contenteditable divs instead of input/textarea elements. The fillInput function now detects contenteditable and uses textContent instead of value property. Also fixes build script to use correct tsconfig path. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Expand special TLDs from 18 to 65 (UK, AU, NZ, JP, CN, BR, IN, ZA, MX, KR, SG, HK, TW, IL) - Add ARIA attributes to options page for screen reader support - Add Content Security Policy to manifest - Sync package.json version in bump-version script Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add psl package for comprehensive TLD support - Add esbuild for bundling utils.js with dependencies - Add PSL type declarations - Add PSL-specific tests for .com.cn, .org.nz, .or.jp, etc. - Bundle size: 159KB (includes full PSL data) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create ESM bundle (utils.js) for background.js imports - Create IIFE bundle (utils-content.js) for content scripts - Fixes "does not provide an export" error in service worker Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show "Please refresh the page" notification when content script isn't loaded (page was open before extension install/reload) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- PSL integration for comprehensive TLD support - ESM/IIFE dual bundling for service worker compatibility - Improved error handling for content script not loaded Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. WalkthroughRenames MailFiller to Clean-Autofill, migrates JS to a TypeScript Changes
Sequence Diagram(s)sequenceDiagram
participant User as Browser (User)
participant BG as Background (service worker)
participant Storage as chrome.storage.sync
participant Utils as CleanAutofillUtils
participant Content as Content Script (page/frame)
User->>BG: click extension icon
BG->>Storage: get(emailDomain)
Storage-->>BG: emailDomain or null
BG->>Utils: extractMainDomain(tab.url)
Utils-->>BG: siteDomain
BG->>BG: construct email (userDomain@siteDomain)
BG->>Content: message { action: "fillEmail", email }
par same-origin frames
Content-->>Content: find input (focused/lastFocused/email/text)
Content-->>Content: fillInput, dispatch events
end
Content-->>BG: { success: true } or { success: false, error }
BG->>User: show notification / open options
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This pull request represents a complete migration from JavaScript to TypeScript with comprehensive testing infrastructure and modern development tooling. The changes rename the extension from "MailFiller" to "Clean-Autofill" and introduce 119 tests with 98%+ coverage.
Changes:
- Complete TypeScript migration with strict mode enabled and organized source structure (src/)
- Addition of 119 comprehensive tests using Bun test runner with DOM support via happy-dom
- Integration of modern tooling: Biome for linting/formatting, Husky for pre-commit hooks, and reorganized build scripts in toolkit/ directory
Reviewed changes
Copilot reviewed 36 out of 50 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| toolkit/typescript/tsconfig.json | TypeScript configuration with ES2020 target, strict mode, and Chrome types |
| toolkit/biome/biome.json | Biome linting and formatting configuration with project-specific rules |
| toolkit/bun/bunfig.toml | Bun test configuration enabling DOM support via test-setup.ts |
| toolkit/husky/pre-commit | Pre-commit hook running typecheck, lint, and tests |
| toolkit/scripts/*.js | Build, pack, validate, and version management scripts (moved from scripts/) |
| src/utils.ts | Shared utilities with PSL integration for domain extraction |
| src/utils.test.ts | Comprehensive tests for utility functions (87 tests) |
| src/options.ts | Options page logic migrated to TypeScript |
| src/options.test.ts | Tests for options page validation and domain handling |
| src/options.html | Updated with accessibility attributes and module script loading |
| src/content.ts | Content script for email filling migrated to TypeScript |
| src/content.test.ts | Tests for content script field detection and filling |
| src/background.ts | Service worker migrated to TypeScript with ES module imports |
| src/background.test.ts | Tests for background email generation logic |
| src/types/*.ts | Type definitions for the extension and PSL library |
| src/test-setup.ts | Test environment setup with happy-dom |
| package.json | Updated with TypeScript tooling, testing framework, and build scripts |
| manifest.json | Updated to use ES modules for service worker and utils-content.js for content scripts |
| .gitignore | Added rules to ignore compiled .js files in src/ |
| .github/workflows/*.yml | Updated CI/CD workflows for TypeScript build process |
| docs/* | Documentation updates reflecting new structure and naming |
Comments suppressed due to low confidence (8)
src/options.html:259
- The options.html file loads utils.js and options.js as ES modules, but these script tags won't work correctly because the manifest specifies that the service worker uses ES modules (type: "module"), but content scripts and options pages need different handling. The options.html should load the compiled JavaScript files from the dist/ directory, not the source TypeScript files.
toolkit/scripts/validate.js:127 - The validation script attempts to check .js files in src/ directory for console.log statements, but after the TypeScript migration these files are .ts files, not .js files. The script should check .ts files instead.
toolkit/scripts/validate.js:140 - The validation script checks for TODO comments in .js files in src/, but after the TypeScript migration these files are now .ts files. The script should check .ts files instead.
toolkit/scripts/validate.js:79 - The validation script checks for .js files in the src/ directory, but after the TypeScript migration, these files no longer exist in src/. The script should either check for .ts files in src/ or check for the compiled .js files in dist/ directory after the build process.
docs/README.md:61 - The documentation mentions "tools/" directory but the actual directory in the project is "toolkit/". This inconsistency could confuse developers. The documentation should reference the correct "toolkit/" directory path.
.claude/AGENTS.md:156 - The AGENTS.md documentation references "tools/" directory instead of the correct "toolkit/" directory. This appears in multiple places in the file structure section and should be corrected to "toolkit/" for consistency with the actual project structure.
.claude/AGENTS.md:153 - The AGENTS.md file structure shows .js extensions for source files in src/ directory, but after the TypeScript migration these should be .ts files. The documentation should correctly show background.ts, content.ts, options.ts, etc. in the src/ directory, and note that the compiled .js files are in dist/.
docs/README.md:60 - The file structure documentation lists files in src/ with .js extensions (background.js, content.js, options.js), but after the TypeScript migration, these should be .ts files. The documentation should reflect that the source files are TypeScript (.ts) and the compiled output in dist/ are JavaScript (.js).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Copy all necessary files | ||
| cp manifest.json dist/ | ||
| cp background.js dist/ | ||
| cp content.js dist/ | ||
| cp options.html dist/ | ||
| cp options.js dist/ | ||
| cp -r icons dist/ | ||
| cp manifest.json dist/Clean-Autofill/ | ||
| cp src/background.js dist/Clean-Autofill/src/ | ||
| cp src/content.js dist/Clean-Autofill/src/ | ||
| cp src/options.html dist/Clean-Autofill/src/ | ||
| cp src/options.js dist/Clean-Autofill/src/ | ||
| cp -r src/icons/* dist/Clean-Autofill/src/icons/ | ||
|
|
There was a problem hiding this comment.
The workflow attempts to copy .js files from src/ directory, but these files don't exist in src/ after the TypeScript migration. The build script should be run first to compile TypeScript files to the dist/ directory, and then the workflow should package the compiled files from dist/ rather than copying non-existent .js files from src/.
| const cleanDomain = domain.replace(/^@/, ''); | ||
|
|
||
| // Improved domain validation - allows single-char labels | ||
| const domainRegex = /^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z]{2,})+$/; |
There was a problem hiding this comment.
The domain validation regex requires at least 2 characters after the first label (due to the quantifier in the middle group), which would reject valid single-character domain labels like "a.b.com". However, the comment on line 46 says "Improved domain validation - allows single-char labels". The regex should be adjusted to truly allow single-character labels: change the middle group from ([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])? to ([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])? or make the entire middle part optional while allowing single characters.
| "@biomejs/biome": "^2.3.13", | ||
| "@happy-dom/global-registrator": "^20.4.0", | ||
| "@types/chrome": "^0.1.36", | ||
| "esbuild": "^0.27.2", |
There was a problem hiding this comment.
The esbuild version in package.json is 0.27.2, but this version was released in early 2023. As of January 2026 (current date), there are likely several major versions of esbuild available with performance improvements and bug fixes. Consider updating to a more recent version, such as 0.24.x or newer, to benefit from improvements and security fixes.
| "esbuild": "^0.27.2", | |
| "esbuild": "^0.24.0", |
| // IIFE bundle for content scripts (sets globalThis.CleanAutofillUtils) | ||
| execSync('npx esbuild dist/utils.js --bundle --outfile=dist/utils-content.js --format=iife --global-name=CleanAutofillUtils --platform=browser --minify', { cwd: ROOT, stdio: 'inherit' }); |
There was a problem hiding this comment.
The utils-content.js file is referenced in the manifest but the build script generates this file by bundling utils.js with esbuild as an IIFE. However, the content script (content.ts) expects to access CleanAutofillUtils from globalThis, which should be set by utils-content.js. The load order in the manifest is correct (utils-content.js before content.js), but there's a potential issue: the build script should ensure that utils-content.js properly sets the global variable. Review the esbuild configuration to ensure the global-name parameter correctly sets globalThis.CleanAutofillUtils.
| "devDependencies": { | ||
| "@biomejs/biome": "^2.3.13", | ||
| "@happy-dom/global-registrator": "^20.4.0", | ||
| "@types/chrome": "^0.1.36", |
There was a problem hiding this comment.
The @types/chrome package version is pinned to 0.1.36, which appears to be an outdated version. The @types/chrome package has had many updates since version 0.1.x. Consider updating to a more recent version to ensure type definitions match the current Chrome Extension APIs, especially for Manifest V3 features.
| "@types/chrome": "^0.1.36", | |
| "@types/chrome": "^0.0.273", |
.claude/CLAUDE.md
Outdated
| ├── tools/ # Build scripts | ||
| │ ├── build.js # Compiles TS + copies assets to dist/ | ||
| │ ├── pack.js # Creates distribution zip | ||
| │ ├── validate.js # Manifest validation | ||
| │ └── bump-version.js # Version management |
There was a problem hiding this comment.
The CLAUDE.md file references "tools/" directory instead of "toolkit/" in the file structure. All references to the build scripts directory should use "toolkit/" to match the actual project structure.
.github/workflows/build-and-test.yml
Outdated
| "src/background.js" | ||
| "src/content.js" | ||
| "src/options.html" | ||
| "src/options.js" | ||
| "src/icons/icon16.png" | ||
| "src/icons/icon32.png" | ||
| "src/icons/icon48.png" | ||
| "src/icons/icon128.png" |
There was a problem hiding this comment.
The GitHub Actions workflow references source files in the wrong location. The workflow copies files from src/ directory (e.g., src/background.js, src/content.js), but these are TypeScript source files that need to be compiled first. The workflow should run the build script first to generate the compiled JavaScript files in the dist/ directory, then package those files instead of trying to copy non-existent .js files from src/.
| @@ -0,0 +1,35 @@ | |||
| { | |||
| "$schema": "https://biomejs.dev/schemas/2.3.13/schema.json", | |||
There was a problem hiding this comment.
The biome.json configuration uses version 2.3.13 in the schema URL, but the devDependency in package.json specifies ^2.3.13 which could resolve to a higher version. This could lead to inconsistencies if Biome makes breaking changes in minor versions. Consider either pinning the exact version in package.json or using a more flexible schema version in the configuration.
.claude/CLAUDE.md
Outdated
| ├── config/ | ||
| │ ├── biome.json # Biome linter/formatter config | ||
| │ ├── tsconfig.json # TypeScript configuration |
There was a problem hiding this comment.
The CLAUDE.md file incorrectly references "config/" directory for biome.json, tsconfig.json, and husky configurations. According to the actual project structure, these are in "toolkit/" directory (toolkit/biome/, toolkit/typescript/, toolkit/husky/). The documentation should be updated to reflect the correct paths.
| ├── config/ | |
| │ ├── biome.json # Biome linter/formatter config | |
| │ ├── tsconfig.json # TypeScript configuration | |
| ├── toolkit/ | |
| │ ├── biome/ | |
| │ │ └── biome.json # Biome linter/formatter config | |
| │ ├── typescript/ | |
| │ │ └── tsconfig.json # TypeScript configuration |
|
|
||
| - name: Upload extension artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: mailfiller-extension | ||
| path: mailfiller-extension.zip | ||
| name: Clean-Autofill | ||
| path: dist/Clean-Autofill.zip | ||
| retention-days: 30 | ||
|
|
There was a problem hiding this comment.
The GitHub Actions workflow attempts to copy .js files from src/ directory, but after the TypeScript migration, src/ contains only .ts files. The .js files are generated in dist/ after compilation. The workflow needs to run the build script first to compile TypeScript, then package the compiled files from dist/ instead of copying from src/.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/AGENTS.md (1)
114-142:⚠️ Potential issue | 🟡 MinorPath references should be
toolkit/scripts/nottools/.Multiple script path references show
tools/but the actual implementation is attoolkit/scripts/. This affects lines 114, 126, 132, and 138.📝 Suggested fix
-#### 1. Version Management (`tools/bump-version.js`) +#### 1. Version Management (`toolkit/scripts/bump-version.js`) ... -#### 2. Extension Packaging (`tools/pack.js`) +#### 2. Extension Packaging (`toolkit/scripts/pack.js`) ... -#### 3. Comprehensive Validation (`tools/validate.js`) +#### 3. Comprehensive Validation (`toolkit/scripts/validate.js`) ... -#### 4. Build Validation (`tools/build.js`) +#### 4. Build Validation (`toolkit/scripts/build.js`)
🤖 Fix all issues with AI agents
In @.claude/AGENTS.md:
- Around line 148-162: Update the README diagram to match the real project
layout by replacing the incorrect "tools/" entry with the actual path
"toolkit/scripts/"; edit the block that lists top-level folders (the tree under
"Clean-Autofill/") so the line currently showing "├── 📁 tools/ #
Development utilities" becomes "├── 📁 toolkit/ # Development
utilities" and include the nested "├── 📁 scripts/ # developer
scripts" (or combine as "├── 📁 toolkit/scripts/") to reflect the true structure
referenced in the repo and ensure consistency with any other mentions in the
file.
In @.claude/CLAUDE.md:
- Around line 80-110: Update the file tree in .claude/CLAUDE.md to reflect the
new layout: remove or replace references to config/husky and tools/ and add the
new toolkit/ entries (e.g., toolkit/husky and toolkit/scripts), ensuring the
manifest, package, bunfig, config/biome.json and tsconfig.json remain accurate
and that src/ and tools-related build scripts are shown under toolkit/ instead
of tools/. Adjust the ASCII tree lines and comments so the listed paths match
the repository changes (reference the manifest.json, package.json, config/,
toolkit/husky, toolkit/scripts, and src/ entries to locate where to edit).
In @.github/workflows/build-and-test.yml:
- Around line 46-54: The workflow fails because it checks for src/*.js and
packages from src without running the TypeScript build; add an npm run build
step before the required_files check, update the required_files check to look in
dist/ (e.g., dist/background.js, dist/content.js, dist/options.js, dist/icons/*)
and update the packaging step to copy files from dist/ instead of src/; also
ensure the build output includes both utils.js and utils-content.js and that the
packaging copies utils-content.js (the manifest.json references utils-content.js
for the content script) so the artifact matches the compiled outputs.
In @.github/workflows/release-chrome-store.yml:
- Around line 84-97: The packaging step fails because it copies compiled JS
files that don't exist (src/background.js, src/content.js, src/options.js) — the
repo uses TypeScript; add a TypeScript build before packaging and point copies
at the compiled output. Modify the workflow to run the TypeScript build command
(e.g., run the existing npm script like npm run build or npm run pack) prior to
the "Create extension package" block, and then change the cp targets to the
built files (e.g., copy from the build/dist output instead of src/) or simply
run npm run pack which both builds and creates the zip; ensure the step invokes
the correct npm script name used in package.json.
In `@docs/GITHUB_ACTIONS_SETUP.md`:
- Around line 85-86: Update the example command that runs the bump script so it
points to the real location: replace the reference to "node
tools/bump-version.js patch" with the correct path "node
toolkit/scripts/bump-version.js patch" wherever the docs show running
bump-version.js (look for the text "bump-version.js" and the example command).
Ensure the example matches the project structure used elsewhere so readers can
run the script successfully.
In `@docs/README.md`:
- Around line 52-69: Update the README tree to reflect the actual scripts
location: replace the tools/ node (and its children build.js, pack.js,
validate.js, bump-version.js) with toolkit/scripts/ so the diagram matches the
repo (referencing the Clean-Autofill tree in README.md and the script filenames
build.js, pack.js, validate.js, bump-version.js); alternatively add a brief note
under the diagram that the scripts live in toolkit/scripts/ to avoid confusion.
In `@src/content.test.ts`:
- Around line 24-115: Tests currently recreate pure functions locally
(isInputField, isElementVisible, findEmailFields, findTextFields) instead of
using the production implementations; export these pure utilities from
content.ts (or move them into a new utilities module) as named exports and
update the test file to import isInputField, isElementVisible, findEmailFields,
and findTextFields from that module, then remove the duplicated local function
copies from the test; if content.ts mixes side effects, extract the pure logic
into separate exported functions to keep production behavior unchanged while
enabling tests to import the real implementations.
In `@src/content.ts`:
- Around line 197-238: isElementVisible incorrectly uses the global window and
document for style and viewport checks which breaks for elements inside
same-origin iframes; update isElementVisible to get the element's own document
and view (use element.ownerDocument and element.ownerDocument.defaultView,
falling back to the global window if null) and use that view for
getComputedStyle calls and its innerWidth/innerHeight for viewport checks, and
when walking parents use element.ownerDocument.body (and compute styles from the
element's document) so visibility checks (htmlElement.getBoundingClientRect,
getComputedStyle, viewport comparisons and parent traversal) are performed in
the element's document/window context rather than the top window.
In `@src/options.ts`:
- Around line 8-14: The DOM element lookups in src/options.ts (form,
emailDomainInput, statusDiv, clearButton, previewBox, exampleEmail,
exampleEmail2) use type assertions and can be null; add defensive null checks
after each document.getElementById call (or use a small guard helper) and handle
missing elements by either throwing a clear error message, early-returning to
disable options logic, or disabling related UI interactions so no property
access occurs on null. Ensure subsequent code only runs when the required
elements are non-null (e.g., guard early in the initialization function or
conditionally branch where those identifiers are used) and include informative
messages mentioning the specific element id when failing fast.
In `@src/types/psl.d.ts`:
- Around line 12-18: The default export in the declaration currently uses an
object literal; change it to a declared binding and export that identifier (or
use export =). For example, declare a const like `const psl: { parse: typeof
parse; get: typeof get; isValid: typeof isValid; }` and then replace `export
default { parse, get, isValid };` with `export default psl;`, making sure
`parse`, `get`, `isValid`, and the `ParsedDomain` type remain exported as shown.
In `@toolkit/scripts/build.js`:
- Around line 176-180: Update the success output in the if (!hasErrors) block in
build.js so it references Bun tooling instead of npm; specifically change the
instructional line that currently says 'Run "npm run pack" to create zip for
distribution' to the appropriate Bun command (or a Bun-centric explanation) and
keep the surrounding console.log calls (the success message and Next steps)
unchanged.
- Around line 81-106: The current export-stripping regexes in the build step
(used when iterating contentScriptFiles and jsFiles, operating on filePath and
the content string) only match `export {}` with a required space, so minified
`export{}` remains; update both regexes that remove empty exports and named
exports to allow optional whitespace after `export` and inside braces (e.g., use
patterns that permit `export{}`, `export {}`, and `export { a }`), and apply
these updated patterns where content is modified before writing via
fs.writeFileSync.
In `@toolkit/scripts/pack.js`:
- Around line 31-34: The code changes the working directory with
process.chdir(distPath) and runs execSync(`zip ...`) but doesn't restore the cwd
if execSync throws; wrap the chdir/execSync block in a try/finally so
process.chdir(rootPath) always runs (even on error). Specifically, move
process.chdir(distPath) before the try, run execSync inside the try, and call
process.chdir(rootPath) in the finally; reference the existing symbols
process.chdir, execSync, distPath, rootPath and zipFileName to locate and update
the pack.js logic.
🧹 Nitpick comments (13)
toolkit/scripts/bump-version.js (1)
72-88: Update the "Next steps" output to include package.json in the commit.The script now updates both
manifest.jsonandpackage.json, but the suggested commit command on line 86 only mentionsmanifest.json.📝 Proposed fix
-console.log(` 1. Commit: git add manifest.json && git commit -m "Bump version to ${newVersion}"`); +console.log(` 1. Commit: git add manifest.json package.json && git commit -m "Bump version to ${newVersion}"`);.gitignore (1)
13-16: Redundant ignore pattern.The pattern
src/types/**/*.json line 15 is redundant sincesrc/**/*.json line 14 already matches all.jsfiles undersrc/, including thesrc/types/subdirectory.🧹 Proposed fix
# Compiled JS (TypeScript outputs to dist/, but ignore any in src/) src/**/*.js -src/types/**/*.jstoolkit/biome/biome.json (2)
32-34: Consider expanding file includes.The current pattern only lints
src/**/*.ts. This excludes:
- Test files (
src/**/*.test.ts) which are TypeScript- Toolkit scripts (
toolkit/**/*.js)If you want Biome to lint test files alongside source files, they're currently included by the pattern. However, if toolkit scripts should also be linted, consider adding them.
📝 Proposed change to also include toolkit scripts
"files": { - "includes": ["src/**/*.ts"] + "includes": ["src/**/*.ts", "toolkit/**/*.js"] }
10-12: ConsidernoExplicitAnyseverity for strict TypeScript.Setting
noExplicitAnyto"warn"allowsanytypes to slip through. Given the PR objective of "strict mode enabled" TypeScript, you may want to escalate this to"error"to enforce full type safety..github/workflows/ci.yml (1)
17-21: Consider pinning Bun to avoid CI drift.Using
latestcan introduce unexpected breakage. Pin to a known version or read from a.bun-versionfile.🔒 Example (using a version file)
- name: Setup Bun uses: oven-sh/setup-bun@v2 with: - bun-version: latest + bun-version-file: .bun-versiondocs/README.md (1)
54-60: Consider clarifying whether structure shows source or build output.The diagram shows
.jsfiles insrc/(e.g.,background.js,content.js), but the PR migrates to TypeScript (.tsfiles). If this represents the build output, consider adding a note or showing the actual source structure with.tsextensions.src/options.test.ts (1)
39-54: Tests duplicate logic instead of importing from source.The test file reimplements
isValidDomain,cleanDomain, andgenerateExampleEmaillocally rather than importing them fromoptions.ts. This means tests could pass even if the actual implementation differs or has bugs.Consider extracting these utilities to a shared module (or from
options.ts) and importing them in tests to ensure you're testing the actual code.src/background.test.ts (1)
16-35: Avoid duplicating production logic/constants in tests.
generateEmailandMESSAGE_TIMEOUTare redefined here, which can drift fromsrc/background.ts. Consider extracting a small pure helper (and constant) into a shared module and importing it in both production and tests.Also applies to: 144-150
src/utils.test.ts (2)
186-206: Timing-based tests may be flaky in CI environments.The
createTimeouttests rely on wall-clock timing (Date.now()comparisons with 45ms threshold). Under load or in slower CI environments, these assertions could intermittently fail.Consider using fake timers or increasing the tolerance margin significantly for more robust CI behavior.
208-273: Debounce tests also depend on real timers.These tests use
setTimeoutwith real delays (50ms wait, 100ms sleep). While generally reliable, they add ~400ms+ to test runtime and could be flaky.For faster and more deterministic tests, consider using Bun's timer mocking capabilities if available, or accept the current approach as pragmatic for this test suite size.
src/content.test.ts (2)
9-9: Unused mock variable.
_mockSendResponseis defined but never used in the tests. Either remove it or use it in relevant test cases.🧹 Proposed fix
-const _mockSendResponse = mock(() => {});
335-362:isElementVisibletest has limited coverage due to happy-dom limitations.The test at line 358-361 acknowledges that happy-dom doesn't fully support
getComputedStyle, so the test only verifies the function returns a boolean without asserting correctness. This reduces test confidence.Consider adding a comment documenting this limitation or investigating if
@happy-dom/global-registratorprovides better support.src/utils.ts (1)
143-145: RedundantclearTimeoutinside thelaterfunction.The
clearTimeout(timeout)on line 144 is unnecessary becauselater()only executes after the timeout has already fired and completed. The timeout ID is already invalid at this point.🧹 Proposed fix
return function executedFunction(this: unknown, ...args: unknown[]) { const later = () => { - clearTimeout(timeout); func.apply(this, args); }; clearTimeout(timeout); timeout = setTimeout(later, wait); } as T;
.github/workflows/build-and-test.yml
Outdated
| "src/background.js" | ||
| "src/content.js" | ||
| "src/options.html" | ||
| "src/options.js" | ||
| "src/icons/icon16.png" | ||
| "src/icons/icon32.png" | ||
| "src/icons/icon48.png" | ||
| "src/icons/icon128.png" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking manifest.json ==="
if [ -f manifest.json ]; then
python3 << 'PYEOF'
import json
with open('manifest.json') as f:
manifest = json.load(f)
print("Background:", manifest.get("background"))
print("Content scripts:", manifest.get("content_scripts"))
print("Icons:", manifest.get("icons", {}))
PYEOF
else
echo "manifest.json not found"
fi
echo ""
echo "=== Checking for TypeScript config ==="
fd -a '(tsconfig|vite\.config|webpack\.config)' . | head -20
echo ""
echo "=== Checking directory structure ==="
fd -a '^(src|dist|build)$' . -type d | head -10
echo ""
echo "=== Looking for background/content/options files ==="
echo "TypeScript files:"
fd -a '\.(background|content|options)\.(ts|tsx)$' . | head -20
echo ""
echo "JavaScript files:"
fd -a '\.(background|content|options)\.js$' . | head -20
echo ""
echo "HTML files:"
fd -a 'options\.html$' . | head -10
echo ""
echo "=== Checking icon locations ==="
fd -a 'icon(16|32|48|128)\.png$' . | head -20Repository: ZAAI-com/MailFiller
Length of output: 700
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Reading workflow file ==="
if [ -f .github/workflows/build-and-test.yml ]; then
cat -n .github/workflows/build-and-test.yml | sed -n '40,110p'
else
echo "Workflow file not found"
fiRepository: ZAAI-com/MailFiller
Length of output: 3115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking package.json scripts ==="
if [ -f package.json ]; then
python3 << 'PYEOF'
import json
with open('package.json') as f:
pkg = json.load(f)
if 'scripts' in pkg:
print("Build scripts:")
for name, cmd in pkg.get('scripts', {}).items():
if 'build' in name.lower() or 'compile' in name.lower():
print(f" {name}: {cmd}")
PYEOF
fi
echo ""
echo "=== Checking if src/*.js exists ==="
ls -la src/*.js 2>/dev/null || echo "No src/*.js files found"Repository: ZAAI-com/MailFiller
Length of output: 267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking build script ==="
if [ -f toolkit/scripts/build.js ]; then
cat -n toolkit/scripts/build.js
else
echo "Build script not found"
fiRepository: ZAAI-com/MailFiller
Length of output: 7958
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Looking for TypeScript source files ==="
fd -a '\.(background|content|options)\.(ts|tsx)$'
echo ""
echo "=== Looking for HTML source files ==="
fd -a 'options\.html$'
echo ""
echo "=== Checking src/ directory contents ==="
if [ -d src ]; then
find src -type f | sort
else
echo "src/ directory does not exist"
fi
echo ""
echo "=== Checking if icons exist ==="
ls -la src/icons/ 2>/dev/null || echo "src/icons/ not found"Repository: ZAAI-com/MailFiller
Length of output: 986
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking root directory for source files ==="
ls -la | grep -E '\.(ts|tsx|js|html)$' || echo "No root source files found"
echo ""
echo "=== Full directory tree (limited depth) ==="
tree -L 2 -a 2>/dev/null || find . -maxdepth 2 -type d | head -30Repository: ZAAI-com/MailFiller
Length of output: 1094
Workflow missing build step and checking wrong directory for compiled files.
The required_files check (lines 46-54) and packaging step (lines 73-76) look for src/*.js files, but TypeScript sources (src/*.ts) compile to dist/ via the build script. Without a build step before the check, the workflow will fail immediately. Update the workflow to:
- Add a build step before the required_files check:
npm run build - Check for files in
dist/directory instead ofsrc/ - Copy from
dist/instead ofsrc/in the packaging step
Additionally, manifest.json expects utils-content.js as a content script (line 2 of manifest), but the workflow doesn't copy this file. The build script generates both utils.js (ESM for background worker) and utils-content.js (IIFE for content scripts).
🤖 Prompt for AI Agents
In @.github/workflows/build-and-test.yml around lines 46 - 54, The workflow
fails because it checks for src/*.js and packages from src without running the
TypeScript build; add an npm run build step before the required_files check,
update the required_files check to look in dist/ (e.g., dist/background.js,
dist/content.js, dist/options.js, dist/icons/*) and update the packaging step to
copy files from dist/ instead of src/; also ensure the build output includes
both utils.js and utils-content.js and that the packaging copies
utils-content.js (the manifest.json references utils-content.js for the content
script) so the artifact matches the compiled outputs.
Summary
Complete migration to TypeScript with comprehensive testing infrastructure and modern development tooling:
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.