2

I have a React component that is part of a React/Apollo Booking app. I store all the bookings in a SQL database that is hooked up with React-Apollo in between. Before the customer starts the payment process the database is sent temporary bookings to "hold" certain times and restrain other customers from booking the same time. And if a customer refreshes/leave the page I wanna remove those temp bookings from the Apollo DB.

The work flow goes like:

  1. Select times to book(and here I add the booking to the DB).
  2. List the customers bookings for review and add extra features.
  3. Payment process

So the issue I have is that if a customer leaves on step 2, then I want to remove those booking from the DB. Can this be done like with the code below or do I need put like a timer on these temp bookings and remove after certain time from the DB if certain criteria is not met?

I have read several topics on the subject and tryed suggested solutions. I am guessing the call to the DB is asynchorone or the fact that props are involved are the issue and that is why it's not working. 

componentDidMount() {
    window.addEventListener('beforeunload', function () {
      this.props.tempBookings.forEach(booking => {
        removeBookingFromDB(booking.id)
      })
    }, false);
  }

  componentWillUnmount() {
    window.addEventListener('beforeunload', function () {
      this.props.tempBookings.forEach(booking => {
        removeBookingFromDB(booking.id)
      })
    }, false)
  }
Sebastian Berg
  • 69
  • 1
  • 11
  • What do you mean by *database*? Is your Apollo store persisted between page reloads? – trixn Mar 10 '20 at 16:44
  • Updated the question, but its a postgres database and before the customers start the payment process, the bookings are stored in the database as bookings. [1. selected times to book] -> [2. review your bookings and extra features] -> [3. payment process] So if a customer leaves the process on step 2, by either reloading the page or leaving the page entirely. I want those booking to be removed from the DB. – Sebastian Berg Mar 10 '20 at 17:55
  • You should be aware that requests being made in the `beforeunload` handler are not guaranteed to complete successfully. So you can't rely solely on that and you will have to implement a fallback anyways to put a timeout on that booking reservations. – trixn Mar 10 '20 at 18:00
  • Ok, good point. But the issue I am having is that I don't get the beforeunload part is not working at all for me. – Sebastian Berg Mar 10 '20 at 18:04
  • What happens in `removeBookingFromDB`? Did you make sure it is actually called? Also you probably meant to call `removeEventListener` in `componentWillUnmount `. – trixn Mar 10 '20 at 18:05
  • Done some more testing componentDidMount() { window.addEventListener('beforeunload', function () { console.log('this will run') this.props.tempBookings.forEach(booking => { console.log('this part does not run') removeBookingFromDB(booking.id) }) }, false); } – Sebastian Berg Mar 10 '20 at 20:11
  • Ah I think I spotted the issue. In the callback you access `this` but the function is not bound to it. You can prevent this by using an arrow function. – trixn Mar 10 '20 at 20:18

1 Answers1

2

The function you provide as a callback for the beforeunload event is not bound to this. Use an arrow function instead:

window.addEventListener('beforeunload', () => {
    this.props.tempBookings.forEach(booking => {
        removeBookingFromDB(booking.id)
    })
}, false);

To correctly remove the listener on unmount you need to bind the handler to the component instance so that you can reference it in componentWillUnmount:

class MyComponent extends React.Component {

    // must be an arrow function
    removeBookings = () => {
        this.props.tempBookings.forEach(booking => {
            removeBookingFromDB(booking.id)
        })
    }

    componentDidMount() {
        window.addEventListener('beforeunload', this.removeBookings, false);
    }

    componentWillUnmount() {
        window.removeEventListener('beforeunload', this.removeBookings, false)
    }
}
trixn
  • 15,761
  • 2
  • 38
  • 55
  • Yes, that change got it working! However I could not use a class method inside a lifecycle event so I had to define the function twice inside the lifecycle methods, but now it does work, thanks for the help! – Sebastian Berg Mar 10 '20 at 21:56
  • @SebastianBerg Defining it twice will not have the desired effect. `removeEventListener` expects a reference to the same function you registered before. Not removing the handler might result in a memory leak and warnings in the console, when unloading the page. Note that I used an arrow function to define `removeBookings`. It will not work with a normal function definition unless you bind it to `this` in the constructor of the class with `this.removeBookings = this.removeBookings.bind(this)`. – trixn Mar 10 '20 at 22:02
  • 1
    Yeah you are right, again. Normally I know these things but seems like I just put my head on backwards or something yesterday =) Now it's working fully as intended, tnx alot for your patience and the time you took to help me out here. – Sebastian Berg Mar 11 '20 at 10:38