Skip to content

[dotnet] [bidi] Declare allowed nullable objects in constructors type #15809

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

Merged
merged 2 commits into from
May 28, 2025

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented May 28, 2025

User description

🔗 Related Issues

Fixes #15561

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)
  • New feature (non-breaking change which adds functionality and tests!)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Enhancement


Description

  • Make DTO constructor parameters nullable where allowed by spec

  • Update BiDi event and data records to accept null values

  • Add missing properties to network request data records

  • Improve type safety for optional BiDi fields


Changes walkthrough 📝

Relevant files
Enhancement
11 files
BrowsingContextInfo.cs
Make Children and OriginalOpener parameters nullable in
BrowsingContextInfo
+1/-2     
NavigateCommand.cs
Allow Navigation parameter to be nullable in NavigateResult
+1/-1     
NavigationInfo.cs
Make Navigation parameter nullable in NavigationInfo         
+1/-1     
LogEntry.cs
Allow Text parameter to be nullable in all LogEntry records
+4/-4     
AuthRequiredEventArgs.cs
Make Context and Navigation parameters nullable in
AuthRequiredEventArgs
+1/-1     
BaseParametersEventArgs.cs
Make Context and Navigation parameters nullable in
BaseParametersEventArgs
+1/-1     
BeforeRequestSentEventArgs.cs
Make Context and Navigation parameters nullable in
BeforeRequestSentEventArgs
+1/-1     
FetchErrorEventArgs.cs
Make Context and Navigation parameters nullable in FetchErrorEventArgs
+1/-1     
RequestData.cs
Add Destination and InitiatorType, make HeadersSize nullable in
RequestData
+1/-1     
ResponseCompletedEventArgs.cs
Make Context and Navigation parameters nullable in
ResponseCompletedEventArgs
+1/-1     
ResponseStartedEventArgs.cs
Make Context and Navigation parameters nullable in
ResponseStartedEventArgs
+1/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added the C-dotnet .NET Bindings label May 28, 2025
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    15561 - Fully compliant

    Compliant requirements:

    • Review all entries in the BiDi spec marked with "/ null" and make appropriate types nullable in .NET implementation
    • Update DTO constructor parameters to be nullable where allowed by the spec
    • Ensure proper type safety for optional BiDi fields
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    New Properties

    The PR adds new properties to RequestData record (Destination and InitiatorType) but there's no explanation about their purpose or where they come from in the spec.

    public record RequestData(Request Request, string Url, string Method, IReadOnlyList<Header> Headers, IReadOnlyList<Cookie> Cookies, long? HeadersSize, long? BodySize, string Destination, string? InitiatorType, FetchTimingInfo Timings);

    Copy link
    Contributor

    qodo-merge-pro bot commented May 28, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @nvborisenko nvborisenko merged commit 78955da into SeleniumHQ:trunk May 28, 2025
    10 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [🚀 Feature]: [dotnet] [bidi] Reevaluate nullable parameters in DTO constructors
    2 participants