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

will-destroy does not invoke re-rendering #18

Open
LordCHTsai opened this issue Dec 5, 2019 · 5 comments
Open

will-destroy does not invoke re-rendering #18

LordCHTsai opened this issue Dec 5, 2019 · 5 comments

Comments

@LordCHTsai
Copy link

This happened when I tried to write a HOC with a parent passing a function to its child, and the function gets called when the child is inserted and going to be destroyed. I made an integration test to demonstrate the issue.

import Component from '@glimmer/component';
import { click, find, render } from '@ember/test-helpers';
import { module, test } from 'qunit';
import { action } from '@ember/object';
import hbs from 'htmlbars-inline-precompile';
import { setupRenderingTest } from 'ember-qunit';
import { tracked } from '@glimmer/tracking';

module('Integration | Modifier | will-destroy', hooks => {
  setupRenderingTest(hooks);

  test('it should invoke UI re-rendering when changing tracked properties', async function(assert) {
    this.owner.register(
      'component:parent-component',
      class extends Component {
        @tracked text = '';

        get message() {
          return this.text;
        }

        @action changeText(text) {
          this.text = text;
        }
      }
    );

    this.owner.register(
      'template:components/parent-component',
      hbs`
        <div data-dummy>{{this.message}}</div>
        {{yield (hash changeText=this.changeText)}}
      `
    );

    this.set('show', true);

    await render(
      hbs`
        <ParentComponent as |parent|>
          {{#if show}}
            <div
              {{did-insert (fn parent.changeText "Hello")}}
              {{will-destroy (fn parent.changeText "World")}}
            >
            </div>
          {{/if}}
          <button data-button {{on "click" (fn parent.changeText "World")}}>Change Text</button>
        </ParentComponent>
      `
    );

    // did-insert invokes re-rendering correctly, now the message is "Hello".
    assert.strictEqual(find('[data-dummy]').innerText, 'Hello'); 

    // trigger destroying. should change text.
    this.set('show', false);

    // will-destroy does not invoke re-rendering. message supposed to be "World".
    assert.strictEqual(find('[data-dummy]').innerText, 'Hello'); 

    await click('[data-button]');
    // if the changeText function is called by other ways, it works seamlessly.
    assert.strictEqual(find('[data-dummy]').innerText, 'World');
  });
});

Also attached some dependencies info here.

"devDependencies": {
  "@ember/optional-features": "^0.7.0",
  "@ember/render-modifiers": "^1.0.2",
  "@glimmer/component": "^0.14.0-alpha.13",
  "ember-cli": "~3.13.1",
  "ember-cli-babel": "^7.7.3",
  "ember-source": "~3.13.2"
},
@pzuraq
Copy link

pzuraq commented Dec 6, 2019

You need to add an await settled(); call right after this.set('show', false);

import { settled } from '@ember/test-helpers';

     // trigger destroying. should change text.
    this.set('show', false);
    await settled();
     

This is because setting the state does not synchronously update anything, it schedules a rerender that we must wait for.

@LordCHTsai
Copy link
Author

Thank you @pzuraq! I added an await settled();, but the result is the same.

    ...
    // did-insert invokes re-rendering correctly, now the message is "Hello".
    assert.strictEqual(find('[data-dummy]').innerText, 'Hello');

    // trigger destroy. should change text.
    this.set('show', false);
    await settled();

    // will-destroy does not invoke re-rendering. message supposed to be "World".
    assert.strictEqual(find('[data-dummy]').innerText, 'Hello');

    await click('[data-button]');
    // if the changeText function is called by other ways, it works seamlessly.
    assert.strictEqual(find('[data-dummy]').innerText, 'World');
    ...

Also, I created this test case is because I had this issue while developing components in the app, but not having the issue while writing tests. The test case here is just a proof of concept that this issue exists.

@pzuraq
Copy link

pzuraq commented Dec 6, 2019

Ah, for sure. Can you PR this test case to this repo? That way we can add it permanently to make sure that this doesn't happen again.

@LordCHTsai
Copy link
Author

Added a PR to demo the issue. Modified the test case to be simpler without Glimmer. #19

@hjdivad
Copy link
Member

hjdivad commented Dec 17, 2019

@pzuraq see #19 (comment)

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

3 participants