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

Support template depency stubbing #88

Closed

Conversation

RomkeVdMeulen
Copy link
Contributor

See #20. Replaces aurelia/loader#27

I wasn't able to get the test to work: the template registry entry isn't being rebuilt during the second run. I'm guessing the entry is being cached somewhere. Some help with this would be much appreciated.

@RomkeVdMeulen
Copy link
Contributor Author

This builds on #87 and may need to be rebased when that gets merged.

@RomkeVdMeulen
Copy link
Contributor Author

RomkeVdMeulen commented Feb 22, 2019

This is starting to get somewhat urgent for us: we've got over 400 tests using aurelia-testing by now, and every so often adding a new test for a composed component will start breaking existing tests. We need better isolation. If there's anything I can do to help speed up adoption of this change, let me know.

/cc @EisenbergEffect @bigopon

@bigopon
Copy link
Member

bigopon commented Feb 22, 2019

@RomkeVdMeulen Can you add me to collaborator of your repo? I wanna add some more tests to it before we progress. Or you can also do it, to make it closer to your real world app. Those tests should be related to compose scenario, html only element, absolute url resolution

@RomkeVdMeulen
Copy link
Contributor Author

@bigopon That sounds good. Could you make a start so I can get an idea for what you have in mind? I'll add any scenarios I can think of.

@bigopon
Copy link
Member

bigopon commented Feb 22, 2019

<template>
  <require from="./a.html"></require>
</template>

In test:

StageComponent
  .inView('<compose view="path/to/html-only-above.html"></compose>')
  .inView('<compose view-model="path/to/some-view-model-with-require"></compose>')
  .inView('<compose view-model="path/to/some-view-model-with-absolute-url.html"></compose>')

There could be more scenarios, but those are probably the basic, related to <compose/>, there should be the same test for absolute url for normal custom element. Also if possible, testing against test app with router would be nice, though we can use compose for that. You can finish compose, and we can comeback for router later.

@RomkeVdMeulen
Copy link
Contributor Author

RomkeVdMeulen commented Feb 22, 2019

Wouldn't a separate PR be better then? I'm hoping this feature can be rolled out soon so I can start properly isolating our own tests.

@bigopon
Copy link
Member

bigopon commented Feb 22, 2019

then let's see what @EisenbergEffect 'd say

cc @zewa666 @fkleuver

@EisenbergEffect
Copy link
Contributor

We don't need to add all the tests now. We can do a follow-up PR. But, did we get the one initial test working or is that broken?

@RomkeVdMeulen
Copy link
Contributor Author

    it('prevents the matching dependency from being loaded even if a template <require>s it', async () => {
      stubTemplateDependency('dist/test/test/resources/my-component');

      const component = await StageComponent
        .withResources('dist/test/test/resources/my-parent-component')
        .inView('<my-parent-component></my-parent-component>');
      await component.create(bootstrap);

      let subComponent = component.element.firstElementChild as HTMLElement;
      expect(subComponent.tagName.toLowerCase()).toEqual('my-component');
      expect(subComponent.innerHTML.trim()).toEqual('');

      await component.dispose();

      resetStubbedTemplateDependencies();
      await component.create(bootstrap);

      subComponent = component.element.firstElementChild as HTMLElement;
      expect(subComponent.innerHTML.trim()).not.toEqual('');

      await component.dispose();
    });

Still broken. I wanted the test to show that if you reset the stubbed dependencies and then load the same component again, the subcomponent will be loaded. But when the parent component is loaded again, the template registry entry isn't being rebuilt. My guess is that the entry with stubbed dependencies remains cached somewhere, but I don't know enough about templating / loading internals to know where to look.

@EisenbergEffect
Copy link
Contributor

@bigopon Would you be able to help @RomkeVdMeulen track down this issue. If we can get that fixed, then we can merge this and get it out. After that we can follow up with additional tests for more scenarios. This one seems pretty critical for real use though.

@bigopon
Copy link
Member

bigopon commented Feb 23, 2019

@RomkeVdMeulen Sorry I thought you already got that test working. I'll have a look

Copy link
Member

@zewa666 zewa666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good first step towards shallow rendering like used with Enzyme. I'd just wonder what happens If you stub twice. Might throw an error that the template is already stubbed. This may happen when using a beforeEach plus another nested one.

@RomkeVdMeulen
Copy link
Contributor Author

Superseded by #89

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

Successfully merging this pull request may close these issues.

4 participants