Discussion icon indicating copy to clipboard operation
Discussion copied to clipboard

How should AJAX requests be handled when a component unmounts?

Open GirlBossRush opened this issue 9 years ago • 4 comments

Consider the following, a two page application featuring "home" and "profile". The profile page requires an AJAX request to populate the template.

http://jsfiddle.net/nps44dk9/2/

If the user navigates to the profile page and navigates back to the homepage before the AJAX request completes, this._data is undefined and an error will occur. The profile page component is unmounted before the request completes.

In this small example the profile property could instead be placed on the root data instead. But what design is appropriate for a larger application? More complex components like forms that perform server-side validation make storing everything on root difficult.

var ValidatedField = {
  computed: {
    validText: function () {
      return this.isValid ? "Valid" : "Invalid";
    }
  },
  data: function () {
    return {
      isValid: true,
      value: ""  
    };
  },
  methods: {
    validate: function () {
      // Perhaps checking if a username is available...
      $.ajax("/example/foo/bar", {
        success: function (response) {
          // This will error if the user navigates away from the form
          // before the response has finished.
          this.isValid = response;
        }.bind(this)
      });
    }
  },
  template: "<input v-on='change: validate' v-attr='value: value' /> {{validText}}"
};

My team has had some success by monitoring a component's this._data, this._isBeingDestroyed, and this._isDestroyed properties.

// Wraps jQuery-esq AJAX library, preventing late requests from running on unmounted components.
function VueAJAXWrapper (path, options) {
  var componentContext = this;

  $.ajax(path, {
    data: options.data,
    error: function (response) {
      if (!componentContext._isBeingDestroyed && !componentContext._isDestroyed) {
        options.error.call(componentContext, response);
      }
    },
    success: function (response) {
      if (!componentContext._isBeingDestroyed && !componentContext._isDestroyed) {
        options.success.call(componentContext, response);
      }
    },
    type: options.type
  })
}

var SettingsPage = {
  created: function () {
    // Fetch initial profile.
    // this.profile = {...};
  },
  methods: {
    updateProfile: function () {
      // Must be called with component context to know if component is still mounted.
      VueAJAXWrapper.call(this, "user/foobar/edit", {
        type: 'post',
        error: function (response) {
          console.error(response);
        },
        success: function (response) {
          this.profile = response;
        }
      });
    }
  },
  data: function () {
    return {
      profile: null
    };
  },
  template: "..."
};

I'm concerned this approach relies too much on component context. Accessing private component properties seems dangerous without @yyx990803's blessing. Does anyone have any feedback on how they would approach this?

Thank you!

GirlBossRush avatar Jun 26 '15 08:06 GirlBossRush

@TeffenEllis I don't think I'll change _isBeingDestroyed and _isDestroyed in the foreseeable future, so you should be ok with this wrapper. vue-router's data hook uses a similar approach and would handle aborted requests automatically, so hopefully when it's released it could make you rely less on the private properties.

yyx990803 avatar Jun 27 '15 20:06 yyx990803

@yyx990803,

Thanks for the insight, this approach solves our AJAX use case.

Having to know the internals of Vue is nuanced in other areas as well. In another component, an audio spectrum visualizer, a similar guard must be put in place to prevent the animationFrame from executing after a component has begun to unmount.

var AudioSpectrumVisualizer = {
  beforeDestroy: function () {
    global.cancelAnimationFrame(this.animationFrame);
  },
  computed: function () {
    this.canvas = this.$$.canvas.getContext('2d');
  },
  data: function () {
    return {
      animationFrame: null,
      canvas: null
    };
  },
  methods: {
    drawFrequency: function (frequency) {
      // ...
    },
    render: function () {
      if (this.audio.status !== "playing") {
        return;
      }

      var frequencyDomain = new global.Uint8Array(this.audio.analyser.frequencyBinCount);

      // Draw frequencies on canvas.
      frequencyDomain.forEach(this.drawFrequency);

      // Repeat rendering for next "moment" in audio. 
      this.animationFrame = global.requestAnimationFrame(this.render);
    }
  },
  props: [
    'audio'
  ],
  template: "<canvas v-el='canvas'></canvas>"
};

For every setTimeout, there must be a clearTimeout in beforeDestroy, this holds true for requestAnimationFrame and cancelAnimationFrame. Performing this.$el.addEventListener creates the same issue. The docs briefly touch on this behavior.

When a ViewModel is destroyed, all event listeners are automatically removed. You don’t need to worry about cleaning it up yourself.

It would be great to see a snippet in best practices to clarify this how less automatic operations should be cleaned up.

Event listeners attached to this.$el and asynchronous methods are not managed by Vue and must be removed in the beforeDestroy to prevent operations on an destroyed component.

var Component = {
  beforeDestroy: function () {
    this.$el.removeEventListener(this.handler);
    clearTimeout(this.timeout);
  },
  compiled: function () {
    // Anti-pattern, use `v-on` directive instead.
    if (this.shouldHandleClick) {
      this.$el.addEventListener("click", this.handler);
    }
  },
  data: function () {
    return {
      timeout: null
    };
  },
  methods: {
    handler: function () {
      console.log("Manually added event handler executed!");
    },
    delayedHandler: function () {
      this.timeout = setTimeout(function () {
        console.log("Delayed handler executed!");
      }, 1000);
    }
  },
  props: [
    'should-handle-click'
  ]
}

Cheers.

GirlBossRush avatar Jun 30 '15 08:06 GirlBossRush

Yep, I will add this to the best practices section in the guide.

yyx990803 avatar Jun 30 '15 14:06 yyx990803

@yyx990803 Was this ever documented anywhere?

ffxsam avatar Feb 09 '18 18:02 ffxsam