zouyaoji / vue-cesium

🎉 Vue 3.x components for CesiumJS.
https://zouyaoji.top/vue-cesium
MIT License
1.53k stars 325 forks source link

Destroying one of many vc-viewer instances causes Cesium to crash #58

Closed alicjamusial closed 4 years ago

alicjamusial commented 4 years ago

[BUG 反馈] Destroying one of many vc-viewer instances causes Cesium to crash

浏览器版本号

Firefox & Chrome

Vue 版本号

2.6.11

组件库版本号

2.1.0

现象描述

Destroying one of many vc-viewer instances causes Cesium to crash. 
It is caused by line 983 in https://github.com/zouyaoji/vue-cesium/blob/master/src/components/viewer/Viewer.vue - the Cesium instance is being destroyed when it should not - there can be other elements using it.

完整异常信息

Cesium just stops working.

在线示例 / 仓库 URL

I'll add later.

复现用例

I'll add later.

预期输出

Cesium will still work, so other vc-viewer instances could be created.

实际输出

Cesium doesn't work.
zouyaoji commented 4 years ago

Hi, @alicjamusial , Do you mean you added multiple vc-viewers in one page? Then destroy one of them, causing Cesium to crash?

alicjamusial commented 4 years ago

Hi, @alicjamusial , Do you mean you added multiple vc-viewers in one page? Then destroy one of them, causing Cesium to crash?

Yes, exactly.

zouyaoji commented 4 years ago

How many vc-viewer did you add? I have time to test it.

alicjamusial commented 4 years ago

Two is enough to cause the crash (you should see Cesium errors in the console after removing one of them).

zouyaoji commented 4 years ago

Hello, I cannot reproduce the issue. Hello, This is an example of my test: vue-cesium-test.zip

alicjamusial commented 4 years ago

Hello! Could you please change your main code to:

<template>
  <div class="hello">
    <div class="tool">
      <button @click="DestroyViewer1">Destroy Viewer1</button>
      <button @click="DestroyViewer2">Destroy Viewer2</button>
    </div>
    <vc-viewer v-if="view1" ref="viewer1"></vc-viewer>
    <vc-viewer v-if="view2" ref="viewer2"></vc-viewer>
  </div>
</template>

<script>
export default {
  name: "HelloWorld",
  props: {
    msg: String
  },
  data() {
    return {
      view1: true,
      view2: true
    };
  },
  mounted() {
    this.$refs.viewer1.createPromise.then(({ viewer }) => {
      this.viewer1 = viewer;
    });
    this.$refs.viewer2.createPromise.then(({ viewer }) => {
      this.viewer2 = viewer;
    });
  },
  methods: {
    DestroyViewer1() {
      this.view1 = false;
    },
    DestroyViewer2() {
      this.view2 = false;
    }
  }
};
</script>

In that scenario - after removing one of the viewer using button, the other crashes. Does it work for you now?

zouyaoji commented 4 years ago

Well, if the v-if is used to destroy the vc-viewer, it will cause problems. Because the vc-viewer component will remove the CesiumJS loaded on the current page by default when it is destroyed.see: vc-viwer.vue

I thought that CesiumJS should be removed by default when destroying the vc-viewer. In your case, this may not be appropriate. Can I add a vue prop to let the user decide whether to remove CesiumJS when the vc-viewer is destroyed? Or do you have better suggestions?

alicjamusial commented 4 years ago

Yeah, it's probably not appropiate in every case that requires many vc-viewer instances being displayed and hidden in application logic. Letting user decide via prop seems fair enough :)

zouyaoji commented 4 years ago

Please use version 2.1.3, vc-viewer adds a removeCesiumScript prop to meet this demand.

alicjamusial commented 4 years ago

Yay! Thank you :) Will test it today.