Skip to content

Copy of PR #22494: refactor: pass only required fields to useBookingForm and useBookings hooks#2

Open
dakshgup wants to merge 1 commit intomainfrom
copy-pr-22494-devin/1752528664-refactor-booker-hook-event-params
Open

Copy of PR #22494: refactor: pass only required fields to useBookingForm and useBookings hooks#2
dakshgup wants to merge 1 commit intomainfrom
copy-pr-22494-devin/1752528664-refactor-booker-hook-event-params

Conversation

@dakshgup
Copy link
Owner

This is a copy of calcom#22494

Originally by: hbjORbj

refactor: pass only required fields to useBookingForm and useBookings hooks

Summary

Refactored the BookerWebWrapper component to pass only the required fields to the useBookingForm and useBookings hooks instead of passing the entire event object. This optimization reduces memory usage and makes the data flow more explicit.

Changes made:

  • useBookingForm: Now receives only { bookingFields: event.data.bookingFields } instead of the full event.data
  • useBookings: Now receives a filtered event object containing only the fields specified in the IUseBookings interface: id, slug, subsetOfHosts, requiresConfirmation, isDynamic, metadata, forwardParamsSuccessRedirect, successRedirectUrl, length, recurringEvent, schedulingType, and subsetOfUsers

The refactoring maintains the same event structure (with data property) to avoid breaking the hook implementations while significantly reducing the amount of data passed between components.

Review & Testing Checklist for Human

⚠️ YELLOW RISK - Moderate risk due to core functionality impact without end-to-end testing

  • Test the complete booking flow end-to-end - Create a test booking to ensure no runtime errors occur and all functionality works correctly
  • Verify hook interfaces match reality - Check that the useBookingForm and useBookings hooks don't internally access any event properties beyond what's defined in their interfaces
  • Check for hidden data dependencies - Ensure the extracted fields don't have nested references to other parts of the event object that could cause runtime issues

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    BWW["BookerWebWrapper.tsx<br/>(EDITED)"]:::major-edit
    UBF["useBookingForm.ts<br/>(hook interface)"]:::context
    UB["useBookings.ts<br/>(hook interface)"]:::context
    IUseBookingForm["IUseBookingForm<br/>(interface)"]:::context
    IUseBookings["IUseBookings<br/>(interface)"]:::context
    
    BWW -->|"filtered: { bookingFields }"| UBF
    BWW -->|"filtered: { id, slug, etc. }"| UB
    IUseBookingForm -->|"defines required fields"| UBF
    IUseBookings -->|"defines required fields"| UB
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit
        L3[Context/No Edit]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#ADD8E6
    classDef context fill:#FFFFFF
Loading

Notes

  • Type checking passed: All TypeScript checks pass, confirming the filtered data structure matches the hook interfaces
  • Performance improvement: Reduces memory usage by only passing required fields instead of the entire event object
  • Maintainability: Makes data dependencies explicit and easier to track
  • Session info: Requested by benny@cal.com (@hbjORbj) - Session: https://app.devin.ai/sessions/7197f6bb9f0043a8a65d33e61d386c7d

⚠️ Important: While the interfaces suggest this refactoring is safe, the booking flow should be tested manually to ensure no hidden dependencies exist between the hooks and the full event object.

…okingForm and useBookings hooks

Originally by: hbjORbj

# refactor: pass only required fields to useBookingForm and useBookings hooks

## Summary

Refactored the `BookerWebWrapper` component to pass only the required fields to the `useBookingForm` and `useBookings` hooks instead of passing the entire `event` object. This optimization reduces memory usage and makes the data flow more explicit.

**Changes made:**
- **useBookingForm**: Now receives only `{ bookingFields: event.data.bookingFields }` instead of the full `event.data`
- **useBookings**: Now receives a filtered event object containing only the fields specified in the `IUseBookings` interface: `id`, `slug`, `subsetOfHosts`, `requiresConfirmation`, `isDynamic`, `metadata`, `forwardParamsSuccessRedirect`, `successRedirectUrl`, `length`, `recurringEvent`, `schedulingType`, and `subsetOfUsers`

The refactoring maintains the same event structure (with `data` property) to avoid breaking the hook implementations while significantly reducing the amount of data passed between components.

## Review & Testing Checklist for Human

**⚠️ YELLOW RISK** - Moderate risk due to core functionality impact without end-to-end testing

- [ ] **Test the complete booking flow end-to-end** - Create a test booking to ensure no runtime errors occur and all functionality works correctly
- [ ] **Verify hook interfaces match reality** - Check that the `useBookingForm` and `useBookings` hooks don't internally access any event properties beyond what's defined in their interfaces
- [ ] **Check for hidden data dependencies** - Ensure the extracted fields don't have nested references to other parts of the event object that could cause runtime issues

---

### Diagram

```mermaid
%%{ init : { "theme" : "default" }}%%
graph TD
    BWW["BookerWebWrapper.tsx<br/>(EDITED)"]:::major-edit
    UBF["useBookingForm.ts<br/>(hook interface)"]:::context
    UB["useBookings.ts<br/>(hook interface)"]:::context
    IUseBookingForm["IUseBookingForm<br/>(interface)"]:::context
    IUseBookings["IUseBookings<br/>(interface)"]:::context

    BWW -->|"filtered: { bookingFields }"| UBF
    BWW -->|"filtered: { id, slug, etc. }"| UB
    IUseBookingForm -->|"defines required fields"| UBF
    IUseBookings -->|"defines required fields"| UB

    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit
        L3[Context/No Edit]:::context
    end

    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#ADD8E6
    classDef context fill:#FFFFFF
```

### Notes

- **Type checking passed**: All TypeScript checks pass, confirming the filtered data structure matches the hook interfaces
- **Performance improvement**: Reduces memory usage by only passing required fields instead of the entire event object
- **Maintainability**: Makes data dependencies explicit and easier to track
- **Session info**: Requested by benny@cal.com (@hbjORbj) - Session: https://app.devin.ai/sessions/7197f6bb9f0043a8a65d33e61d386c7d

**⚠️ Important**: While the interfaces suggest this refactoring is safe, the booking flow should be tested manually to ensure no hidden dependencies exist between the hooks and the full event object.
@github-actions
Copy link

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Copy of PR #22494: refactor: pass only required fields to useBookingForm and useBookings hooks". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@dakshgup
Copy link
Owner Author

cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Missing Query State Properties in `useBookings`

The useBookings hook now receives an event object that is missing critical isSuccess, isError, and isPending properties. Previously, the event object included these query state properties, but the refactored code explicitly passes an object containing only the data property. This omission removes essential loading and error state information, which useBookings likely relies on, potentially causing runtime errors or incorrect behavior.

packages/platform/atoms/booker/BookerWebWrapper.tsx#L158-L176

const bookings = useBookings({
event: {
data: event.data
? {
id: event.data.id,
slug: event.data.slug,
subsetOfHosts: event.data.subsetOfHosts,
requiresConfirmation: event.data.requiresConfirmation,
isDynamic: event.data.isDynamic,
metadata: event.data.metadata,
forwardParamsSuccessRedirect: event.data.forwardParamsSuccessRedirect,
successRedirectUrl: event.data.successRedirectUrl,
length: event.data.length,
recurringEvent: event.data.recurringEvent,
schedulingType: event.data.schedulingType,
subsetOfUsers: event.data.subsetOfUsers,
}
: null,
},

Fix in CursorFix in Web


BugBot free trial expires on July 22, 2025
Learn more in the Cursor dashboard.

Was this report helpful? Give feedback by reacting with 👍 or 👎

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR optimizes data flow in the BookerWebWrapper component by implementing a more focused approach to passing data to hooks. Instead of passing the entire event object, it now passes only the specific fields that each hook requires:

  • useBookingForm now only receives bookingFields from event.data
  • useBookings receives a filtered subset of event.data containing only the essential fields (id, slug, subsetOfHosts, etc.)

This change aligns with TypeScript best practices by making data dependencies explicit and reducing memory overhead. The refactoring maintains the existing event structure to ensure backward compatibility.

Confidence score: 4/5

  1. This PR is relatively safe to merge with proper testing as it's a well-structured refactor
  2. High score due to clear type safety, explicit interfaces, and maintaining existing structure while improving code quality
  3. Files needing attention:
    • BookerWebWrapper.tsx: Verify no hidden dependencies exist in the hook implementations
    • End-to-end testing of the complete booking flow is crucial

1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile

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.

1 participant