Skip to content

Conversation

@hmafzal
Copy link
Contributor

@hmafzal hmafzal commented Sep 3, 2025

fix: replace @ts-ignore with proper TypeScript typing in useScript

Summary

Replaced problematic @ts-ignore comments in the useScript hook with proper TypeScript typing. The changes improve type safety while maintaining the same functionality for setting attributes on HTMLScriptElement objects.

Key changes:

  • Replace @ts-ignore with explicit (scriptEl as any) type assertion
  • Improve property existence check from scriptEl[key] === undefined to !(key in scriptEl)
  • Eliminate TypeScript linting violations while preserving runtime behavior

Review & Testing Checklist for Human

  • Verify attribute setting logic: Test that both setAttribute() and direct property assignment paths work correctly with real script attributes (e.g., async, defer, crossorigin)
  • Check property existence logic: Confirm that !(key in scriptEl) behaves identically to scriptEl[key] === undefined for HTMLScriptElement properties - this is the main behavioral change
  • Test end-to-end: Verify that scripts load correctly with various attribute combinations in a real browser environment

Notes

  • The explicit as any cast is more transparent than @ts-ignore but still bypasses type safety for this specific line
  • Build and TypeScript compilation pass successfully
  • This change addresses TypeScript linting violations identified during code review

Link to Devin run: https://app.devin.ai/sessions/b3abc6bba0d74c31a560e682d6617bfc
Requested by: @hmafzal

- Replace @ts-ignore comments with explicit type assertions
- Improve property existence check from undefined comparison to 'in' operator
- Maintain functionality while improving type safety
- Resolves TypeScript linting violations

Co-Authored-By: Hasan Afzal <[email protected]>
@devin-ai-integration
Copy link

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link

@windsurf-bot windsurf-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 To request another review, post a new comment with "/windsurf-review".

} else {
// @ts-ignore
scriptEl[key] = attributes[key];
(scriptEl as any)[key] = attributes[key];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an explicit type assertion is better than @ts-ignore, but consider using a more specific type like (scriptEl as Record<string, any>)[key] instead of any to maintain some type safety while still allowing dynamic property access.

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.

1 participant