Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add missing timer clear methods to ContentScriptContext #1507

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kikutadev
Copy link

Overview

Added missing timer clear methods (clearTimeout, clearInterval, cancelAnimationFrame, cancelIdleCallback) to the ContentScriptContext class. The current class provides corresponding timer creation methods (setTimeout, setInterval, etc.) but lacks methods to clear them, causing TypeScript compilation errors.

Manual Testing

Test with code using ContentScriptContext like:

const timerId = ctx.setTimeout(() => {
  console.log('This should not run');
}, 1000);

ctx.clearTimeout(timerId); // This should work properly

Related Issue

No related issue.

Copy link

netlify bot commented Mar 8, 2025

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit 39779fd
🔍 Latest deploy log https://app.netlify.com/sites/creative-fairy-df92c4/deploys/67cc8a4e44f15c00084cd3b4
😎 Deploy Preview https://deploy-preview-1507--creative-fairy-df92c4.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aklinker1
Copy link
Collaborator

aklinker1 commented Mar 8, 2025

Thanks for the PR, but why not just call clearTimeout or the other clear functions directly?

@kikutadev
Copy link
Author

@aklinker1
Thank you for reviewing my PR.

I understand that directly using global functions like clearTimeout would technically work, but I proposed these additions for the following reasons:

  1. API Consistency: The context already provides methods like setTimeout and setInterval, so developers would naturally expect corresponding clear methods to be available as well.

  2. Documentation Clarity: While the documentation shows examples of using ctx.setTimeout(), it doesn't mention how to clear these timers, which can be confusing for developers. (I personally was confused when I encountered type errors using ctx.clearTimeout(), and ended up creating a d.ts file to wrap wxt/client).

  3. Type Safety: I was concerned that getting a timer ID from ctx.setTimeout() and then trying to use the global clearTimeout might cause type confusion or inconsistencies.

While I acknowledge that not adding these clear methods wouldn't necessarily cause problems, I believe making these additions would create a more intuitive and consistent developer experience. That said, I'm open to your thoughts on the best approach.

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.

2 participants