Skip to content

Conversation

@SimYunSup
Copy link
Contributor

Overview

Add chrome and firefox variable for specific logic. I'm not sure if this implementation is correct, so I'm going to submit it as a Draft PR.

Manual Testing

Related Issue

Related #1652

@netlify
Copy link

netlify bot commented May 6, 2025

Deploy Preview for creative-fairy-df92c4 failed.

Name Link
🔨 Latest commit b986555
🔍 Latest deploy log https://app.netlify.com/projects/creative-fairy-df92c4/deploys/68441736e13bb60008cf3c6e

@SimYunSup SimYunSup force-pushed the feat/chrome-and-firefox branch from 3f36018 to 79bc991 Compare May 25, 2025 10:28
@SimYunSup SimYunSup marked this pull request as ready for review May 25, 2025 10:58
@SimYunSup
Copy link
Contributor Author

@aklinker1 I wanted to find type-safe import.meta.env type guard, but there wasn't.
So I added just Firefox type. Is there any that I have to implement based on issue?

@aklinker1
Copy link
Member

I'm not sure if I like this change.

  • What about safari?
  • Breaking the standards - projects migrating to WXT may not know which variable to use, and projects migrating away from WXT now have more work to do.
    • Counterpoints:
      • They could just install the @wxt-dev/browser package and continue to use it
      • WXT already uses browser, which most projects don't use.
  • I would rather have a single variable include all the types, rather than exporting multiple variables each with their own types.
  • Why add a variable for chrome when the browser variable is already typed using @types/chrome?
  • What happens to this logic when Chrome releases the browser global for compatibility? if (firefox) ... else if (chrome) ... would not work anymore since it would always be firefox.

Let's continue the discussion on #1652 before doing any more work here. I'm not convinced this is the correct solution yet.

@aklinker1
Copy link
Member

Gonna close this given the conversation here: #1652 (comment)

Doesn't look like we'll be adding the chrome and firefox varaibles. Let's open a new PR for whatever is decided.

@aklinker1 aklinker1 closed this Aug 3, 2025
@SimYunSup
Copy link
Contributor Author

@aklinker1 This implemention is changed for automatically generating TypeScript definitions based on data from @types/chrome and @types/firefox-webext-browser as I commented #1652 (comment) . Could I create new PR from this branch?

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