forcedotcom / LightningTestingService

Apache License 2.0
122 stars 35 forks source link

testutil component destruction issue when API versions differ #26

Closed maxwellmstein closed 6 years ago

maxwellmstein commented 7 years ago

cmp.destroy() fails in the clearRenderedTestComponents function if the targeted component is outside of the locker.

The workaround is simple. Inside of testutils change

if(status === "SUCCESS"){
    cmpsToCleanup.push(component.getGlobalId());
    if (renderInto) {

to

if(status === "SUCCESS"){
    if (renderInto) {
         var renderingContainer = $A.getComponent(renderInto);
         cmpsToCleanup.push(renderingContainer.getGlobalId());

then below in the clearRenderedTestComponents

while (cmpsToCleanup.length) {
        var globalId = cmpsToCleanup.shift();
        var cmp = $A.getComponent(globalId);
        cmp.set('v.body', []);
}

There is a scenario where the component is created and fails to be set to the renderInto body that needs to be handled, but this works quick and dirty.

esalman-sfdc commented 7 years ago

Thanks @maxwellmstein for reporting the issue and sharing your approach to work around it.

By the way, is there are reason you aren't moving your code to version 40 to ensure everything is running inside a locker?

maxwellmstein commented 7 years ago

Yeah - for production builds all components will be v.40. I'm using a Redux store and the sweet chrome devtoolbar for it relies on the store being accessible on the window. So I have that one component at v.39 for development.

I think this issue would occur if you created components outside your namespace as well though, right? Say if I wanted to create some standard components to test my custom components interaction with them.

esalman-sfdc commented 7 years ago

Yes, basically when you access a component which isn't in the same locker (different namespace or below version-40), lockerService returns a SecureComponentRef wrapper instead of SecureComponent. SecureComponentRef's interface is alot more restrictive.

I do agree that clearRenderedTestComponents should be updated to support deletion of any component :)

esalman-sfdc commented 7 years ago

Update: For Winter'18, destroy() has been added to SecureComponentRef meaning that the current logic in the test util of LTS should start working as-is for components running without locker or from other namespaces. Related LockerService work-item W-4099006.