Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

calling set on destroyed object #63

Open
Samsinite opened this issue Nov 19, 2019 · 3 comments
Open

calling set on destroyed object #63

Samsinite opened this issue Nov 19, 2019 · 3 comments

Comments

@Samsinite
Copy link

Samsinite commented Nov 19, 2019

I am on 1.6.0, but it appears that this is still an issue on master due to sets being done inside then instead of using yield and letting ember-concurrency cancel it on destroy. While this error seems to also be swallowed by ember-cli-new-version, it appears to be throwing the following error:

"calling set on destroyed object: <app-ember@component:new-version-notifier::ember608>.version = v2.19.1-1-g8c28df4d"

I only noticed this error due to a breakpoint in ember where exceptions are logged. I believe this error would disappear by using ember-concurreny to handle all async operations (since it cancels the operations when an object is destroyed), for example:

      const response = yield fetch(url + '?_=' + Date.now());
      if(!response.ok) { throw new Error(response.statusText); }

      const res = yield response.text();
      const currentVersion = this.get('version');
      const newVersion = res && res.trim();

      if (this.updateNeeded(currentVersion, newVersion)) {
        const message = this.get('updateMessage')
          .replace('{oldVersion}', currentVersion)
          .replace('{newVersion}', newVersion);

        this.setProperties({
          message,
          lastVersion: currentVersion
        });
        this.onNewVersion(newVersion, currentVersion);
      }

      this.set('version', newVersion);

The current source code can be seen at https://github.com/sethwebster/ember-cli-new-version/blob/master/addon/components/new-version-notifier/component.js#L83. If this update is acceptable, I'll happily open a PR.

Thanks for the awesome addon!

@knownasilya
Copy link
Collaborator

@Samsinite totally open for a PR

@Samsinite
Copy link
Author

👍, thanks!

@knownasilya
Copy link
Collaborator

@Samsinite get a chance to write up that fix?

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

No branches or pull requests

2 participants