Skip to content

Conversation

@toofishes
Copy link
Contributor

Overview

#1601 adjusted the typing of defineItem to examine whether defaultValue or fallback were passed, and only then, remove the null from the possible returned value. However, it did not accommodate the case where an init function is passed to prevent a null return, which was previously allowed.

Add a new type signature for this case, and add some tests for type expectations to catch the issue.

Manual Testing

Added type tests that fail before this change, and succeed after.

Related Issue

@netlify
Copy link

netlify bot commented Sep 24, 2025

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit 7cb0da8
🔍 Latest deploy log https://app.netlify.com/projects/creative-fairy-df92c4/deploys/6941e094ebbc55000892778a
😎 Deploy Preview https://deploy-preview-1909--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 project configuration.

@RayGuo-ergou
Copy link
Contributor

just about to create a PR for this :) haha thank you!

@aiktb
Copy link
Contributor

aiktb commented Oct 3, 2025

LOL I didn't see this, I created a similar one

@toofishes
Copy link
Contributor Author

@aklinker1 Is there anything I can do to help get this reviewed and merged?

Copy link
Member

@aklinker1 aklinker1 left a comment

Choose a reason for hiding this comment

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

Nice, hopefully I have enough type-tests that they verify this doesn't break anything lol

Is there anything I can do to help get this reviewed and merged?

Nope, I just had to find time to get to reviewing it. Been very busy these last five months, but I put in a full 8 hours of WXT work today... Might have completely blown off work lol

@aklinker1
Copy link
Member

@toofishes could you merge this with the latest main? I would have done it, but I can't make changes to this PR. PR checks won't run until it's been merged.

wxt-dev#1601 adjusted the typing of `defineItem` to
examine whether `defaultValue` or `fallback` were passed, and only then, remove
the `null` from the possible returned value. However, it did not accommodate
the case where an `init` function is passed to prevent a `null` return, which
was previously allowed.

Add a new type signature for this case, and add some tests for type
expectations to catch the issue.
@toofishes toofishes force-pushed the defineItem-types-init branch from f44bdb8 to 7cb0da8 Compare December 16, 2025 22:43
@toofishes
Copy link
Contributor Author

Rebased on latest main branch - on my end it says two workflows still need approval from a maintainer, but hopefully the others can run!

@aklinker1
Copy link
Member

Thanks, I just had to click the "approve and run" button, but it wasn't showing up. They're all running now, hopefully everything passes...

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 16, 2025

Open in StackBlitz

@wxt-dev/analytics

npm i https://pkg.pr.new/@wxt-dev/analytics@1909

@wxt-dev/auto-icons

npm i https://pkg.pr.new/@wxt-dev/auto-icons@1909

@wxt-dev/browser

npm i https://pkg.pr.new/@wxt-dev/browser@1909

@wxt-dev/i18n

npm i https://pkg.pr.new/@wxt-dev/i18n@1909

@wxt-dev/module-react

npm i https://pkg.pr.new/@wxt-dev/module-react@1909

@wxt-dev/module-solid

npm i https://pkg.pr.new/@wxt-dev/module-solid@1909

@wxt-dev/module-svelte

npm i https://pkg.pr.new/@wxt-dev/module-svelte@1909

@wxt-dev/module-vue

npm i https://pkg.pr.new/@wxt-dev/module-vue@1909

@wxt-dev/runner

npm i https://pkg.pr.new/@wxt-dev/runner@1909

@wxt-dev/storage

npm i https://pkg.pr.new/@wxt-dev/storage@1909

@wxt-dev/unocss

npm i https://pkg.pr.new/@wxt-dev/unocss@1909

@wxt-dev/webextension-polyfill

npm i https://pkg.pr.new/@wxt-dev/webextension-polyfill@1909

wxt

npm i https://pkg.pr.new/wxt@1909

commit: 7cb0da8

@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.08%. Comparing base (2466786) to head (7cb0da8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1909      +/-   ##
==========================================
+ Coverage   75.98%   76.08%   +0.09%     
==========================================
  Files         113      113              
  Lines        3040     3040              
  Branches      685      685              
==========================================
+ Hits         2310     2313       +3     
+ Misses        646      643       -3     
  Partials       84       84              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aklinker1 aklinker1 merged commit 5d0a266 into wxt-dev:main Dec 16, 2025
18 checks passed
@github-actions
Copy link
Contributor

Thanks for helping make WXT better!

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.

4 participants