Memory leak if onopen event received after we close a component.

[sbernard31]

I begin to play with vue-sse and I maybe find an issue.

My code looks like :

export default {
  mounted() {
    console.log("MOUNTED", this.sse);
    this.$sse("api/event", { format: "json" })
      .then((sse) => {
        this.sse = sse; // store sse to close in on Destroy
        console.log("CONNECTED");
        sse.onError((e) => {
          console.error("on error", e);
        });
        sse.subscribe("UPDATED", () => {
          console.log("UPDATED");
        });
      })
      .catch((err) => {
        console.error("Failed to connect to server", err);
      });
  },
  beforeDestroy() {
    console.log("DESTROYED", this.sse ? "sse defined" : "sse undefined");
    this.sse.close();
  },
};

This sounds OK but for a reason I can not explain for now the onopen call take a lot of time before to be called. (I don't know if this is an issue but this reveals what it looks like a real design issue)

Here is the normal use case :

MOUNTED  // I open my vue
CONNECTED // get connected
UPDATED  // received some messages
UPDATED 
UPDATED 
UPDATED 
DESTROYED source defined // close the vue

Now this is the failing use case where I close my view before to get connected

MOUNTED // I open my vue
DESTROYED source undefined  // I close it before the onopen
[Vue warn]: Error in beforeDestroy hook: "TypeError: this.sse is undefined"
CONNECTED // onopen is called after beforeDestroy and so will live "forever"
UPDATED
UPDATED
UPDATED

It seems to me that my usecase is a "classic" one and so unless I missed something there is maybe a design issue ? 🤔
Adapting vue-sse code to get the wrapper object immediatly without waiting the onopen event (like proposed in #7), this resolved my issue.

If there is no issue in the code and if there is a good way to handle this, I guess there is at least an issue in README examples.

[tserkov]

Thank you for clarifying this edge case that has existed for some time. My personal projects called $sse from a component that wasn't subject to rapid create/destroy lifecycles.

The fix, unfortunately, requires heavily changing the existing interface to the library, so I will be working on a v2 for this.

[sbernard31]

Let me know, if there is some code to test or review :-)

[tserkov]

I've published a release candidate for v2, though I still need to update the readme.

https://github.com/tserkov/vue-sse/tree/v2
https://www.npmjs.com/package/vue-sse

I've also created an example application: https://github.com/tserkov/vue-sse-example

This version creates everything @sbernard31 has requested, and includes typescript definitions.

[sbernard31]

I played a little bit with it.
I think the new design fixed this bug.
I like the new way to define default config.

I think maybe we should add a shortcut for Vue.$sse.create({ url: '/your-events-endpoint' }) which looks like Vue.$sse.create( '/your-events-endpoint')

If I well understand there is no more onError callback or I missed something or maybe this is a bug ? If I'm correct, it should be removed from the README

So the code looks like :

<template>
  <div>
    <p v-for="message in messages" :key="message.id">{{ message }}</p>
  </div>
</template>

<script>
export default {
  name: "sse-test",
  data() {
    return {
      messages: [],
    };
  },
  mounted() {
    this.sse = this.$sse
      .create({ url: "/your-events-server" })
      .on("chat", (message) => {
        this.messages.push(message);
      })
      .connect()
      .catch((err) => {
        console.error("Failed to connect to server", err);
      });
  },
  beforeDestroy() {
    this.sse.disconnect();
  },
};
</script>

I didn't find anything about #13 ? I think if the disconnect could be done automatically by default this could be great.
With previous shortcut we could have this simple way for common use cases :

<template>
  <div>
    <p v-for="message in messages" :key="message.id">{{ message }}</p>
  </div>
</template>

<script>
export default {
  name: "sse-test",
  data() {
    return {
      messages: [],
    };
  },
  mounted() {
    this.$sse
      .create("/your-events-server")
      .on("chat", (message) => {
        this.messages.push(message);
      })
      .connect()
      .catch((err) => {
        console.error("Failed to connect to server", err);
      });
  },
};
</script>

[tserkov]

Thank you for your feedback.

I will work to make the first parameter to $create a string (for URL) or object (for config).

I did remove the old onError system, since consumers can simply add it as a handler: mysse.on('error', () => /* react to connection outage */);. This will be updated in the README.

I haven't implemented #13 yet -- it's a complicated thing to do. I will somehow need to associate an instance of SSEClient to a specific component.

[sbernard31]

I will work to make the first parameter to $create a string (for URL) or object (for config).

👍

I did remove the old onError system, since consumers can simply add it as a handler: mysse.on('error', () => /* react to connection outage */);. This will be updated in the README.

👍

it's a complicated thing to do. I will somehow need to associate an instance of SSEClient to a specific component.

Yep I understand let's talk about his in #13.

[sbernard31]

My previous test show that v2 fixed this bug so I guess we can close this bug ?

Do you need an issue for :

I will work to make the first parameter to $create a string (for URL) or object (for config).