-
-
Notifications
You must be signed in to change notification settings - Fork 255
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
feat(zip): Force sources zip and auto source on opera #1014
Conversation
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for the PR. A few small changes, then we'll get this thing merged.
packages/wxt/src/core/zip.ts
Outdated
wxt.config.browser === 'firefox' || | ||
wxt.config.browser === 'opera' || | ||
config?.zip?.alwaysBuildSourcesZip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having this logic here, can we create a wxt.config.zip.zipSources
boolean that gets set to true
or false
inside resolve-config.ts
? That way this code can just be if (wxt.config.zip.zipSources) { ... }
.
This is the pattern I've been following with all config - simplify config as much of it as possible inside resolve-config.ts
so if it's referenced multiple times throughout the code, we don't have to recalculate the value multiple times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, idk why but adding / changing something in the config is really hard? You have to manually add it in like 4 places and typecheck doesn't even throw an error if you forget it at one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, types should throw an error... I guess unless you use optional types, then it won't. I try to use | undefined
in the resolved config type to avoid that.
@wxt-dev/auto-icons
@wxt-dev/i18n
@wxt-dev/module-react
@wxt-dev/module-solid
@wxt-dev/module-svelte
@wxt-dev/module-vue
wxt
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1014 +/- ##
==========================================
- Coverage 82.14% 81.98% -0.16%
==========================================
Files 127 127
Lines 6625 6633 +8
Branches 1103 1101 -2
==========================================
- Hits 5442 5438 -4
- Misses 1169 1181 +12
Partials 14 14 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, this looks good. I updated some of the docs, simplified some code, and added more tests. Gonna merge and release.
Looks good and oof I forgot to remove that console log haha |
Any chance to produce different zip sources depending on the browser? Because a source to build a Firefox extension will be different than an Opera one |
Closes #1012
This pull request introduces new functionality to always create a sources zip file under certain conditions and updates related tests, configurations, and command-line options accordingly.
New Functionality:
packages/wxt/src/types.ts
,packages/wxt/src/core/zip.ts
,packages/wxt/src/core/resolve-config.ts
,packages/wxt/src/cli/commands.ts
) [1] [2] [3] [4] [5]Tests:
packages/wxt/e2e/tests/zip.test.ts
)Configuration:
zipSources
option. (packages/wxt/src/types.ts
,packages/wxt/src/core/utils/testing/fake-objects.ts
) [1] [2]Command-Line Interface:
--sources
to always create sources zip files regardless of the browser. (packages/wxt/src/cli/commands.ts
) [1] [2]