refactor: DerefOpiton default to Some(T) when T impl Default trait #12406
Conversation
✅ Deploy Preview for rspack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the DerefOption<T> type to default to Some(T::default()) instead of None when T implements the Default trait. This change makes it safer to wrap types in DerefOption without changing their default initialization behavior, which helps prevent subtle bugs. The PR also removes an unnecessary T: Debug constraint from the DerefMut implementation.
Key changes:
- Implements a custom
Defaulttrait forDerefOption<T>that createsSome(T::default())whenT: Default - Removes the derived
Defaultimplementation that would have createdNone - Removes unnecessary
T: Debugbound fromDerefMutimplementation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Binary Size-limit
❌ Size increased by 128bytes from 47.72MB to 47.72MB (⬆️0.00%) |
Rsdoctor Bundle Diff AnalysisFound 5 project(s) in monorepo. 📁 react-10kPath:
📦 Download Diff Report: react-10k Bundle Diff 📁 react-1kPath:
📦 Download Diff Report: react-1k Bundle Diff 📁 react-5kPath:
📦 Download Diff Report: react-5k Bundle Diff 📁 romePath:
📦 Download Diff Report: rome Bundle Diff 📁 ui-componentsPath:
📦 Download Diff Report: ui-components Bundle Diff Generated by Rsdoctor GitHub Action |
CodSpeed Performance ReportMerging #12406 will not alter performanceComparing Summary
|
Why
Change DerefOption to default to Some(T) when T implements the Default trait.
Currently, Artifacts implements Default and is initialized by calling Default::default(). If Artifacts is changed to DerefOption, the existing behavior changes subtly: all fields are initialized to None instead of using Artifacts::default().
This difference in default initialization is easy to miss and can lead to subtle bugs. Defaulting DerefOption to Some(T::default()) when T: Default preserves the original behavior and makes the change safer and more intuitive.
Related links
Checklist