Merge pull request #98 from ttsahi/master

Component will not call render after it has been unmounted

.circleci/config.yml

@@ -6,7 +6,7 @@ version: 2
 jobs:
   build:
     docker:
-      - image: circleci/node:8.9
+      - image: circleci/node:8.9-browsers
 
     steps:
       - checkout

.gitignore

@@ -1,3 +1,4 @@
+.idea
 .DS_Store
 node_modules
 *.d.ts

.npmignore

@@ -1,3 +1,4 @@
+.idea
 .DS_Store
 .circleci
 node_modules

index.tsx

@@ -30,6 +30,7 @@ export function react2angular<Props>(
       static get $$ngIsClass() {
         return true
       }
+      isDestroyed = false
       injectedProps: { [name: string]: any }
       constructor(private $element: IAugmentedJQuery, ...injectedProps: any[]) {
         super()
@@ -39,12 +40,15 @@ export function react2angular<Props>(
         })
       }
       render() {
-        render(
-          <Class {...this.props} {...this.injectedProps as any} />,
-          this.$element[0]
-        )
+        if (!this.isDestroyed) {
+          render(
+            <Class {...this.props} {...this.injectedProps as any} />,
+            this.$element[0]
+          )
+        }
       }
       componentWillUnmount() {
+        this.isDestroyed = true
         unmountComponentAtNode(this.$element[0])
       }
     }]

package.json

@@ -7,8 +7,8 @@
   "typings": "index.d.ts",
   "scripts": {
     "build": "npm run clean && npm run lint && tsc -d -t es2015 && mv ./index.js ./index.es2015.js && tsc -t es5",
-    "clean": "rm -f ./*.d.ts && rm -f ./*.map",
-    "lint": "tslint -p ./tsconfig.json index.tsx test.tsx",
+    "clean": "rimraf ./*.d.ts && rimraf ./*.map",
+    "lint": "tslint --fix -p ./tsconfig.json index.tsx test.tsx",
     "pretest": "npm run build",
     "prepublishOnly": "npm test",
     "test": "karma start --single-run",
@@ -52,7 +52,7 @@
     "@types/prop-types": "^15.5.8",
     "@types/react": "^16.8.1",
     "@types/react-dom": "^16.0.11",
-    "angular-mocks": "^1.7.6",
+    "angular-mocks": "1.6.9",
     "angular-resource": "^1.7.6",
     "browserify": "^16.2.3",
     "jasmine": "^3.3.1",
@@ -68,6 +68,7 @@
     "prop-types": "^15.6.2",
     "react": "^16.7.0",
     "react-dom": "^16.7.0",
+    "rimraf": "^2.6.3",
     "rollupify": "^0.5.1",
     "tslint": "^5.12.1",
     "typescript": "^3.3.1",

test.tsx

@@ -1,4 +1,13 @@
-import { bootstrap, element as $, IAugmentedJQuery, ICompileService, IHttpService, IQService, module } from 'angular'
+import {
+  bootstrap,
+  element as $,
+  IAugmentedJQuery,
+  ICompileService,
+  IComponentOptions, IController,
+  IHttpService,
+  IQService, IScope,
+  module
+} from 'angular'
 import * as angular from 'angular'
 import 'angular-mocks'
 import { $http, $q, $rootScope } from 'ngimport'
@@ -102,12 +111,60 @@ function TestSeven(props: Props) {
   return <p>{props.foo}</p>
 }
 
+interface TestEightProps {
+  onChange: jasmine.Spy,
+  onComponentWillUnmount: jasmine.Spy,
+  onRender: jasmine.Spy,
+  values: string[],
+}
+
+class TestEight extends React.Component<TestEightProps> {
+  render() {
+    this.props.onRender()
+    return this.props.values
+      .map((value, index) => <div key={index}>{value}</div>)
+  }
+
+  componentWillUnmount() {
+    this.props.onComponentWillUnmount()
+    this.props.onChange(this.props.values
+        .map(val => `${val}ss`))
+  }
+}
+
+class TestEightWrapper implements IComponentOptions {
+  bindings = {
+    onComponentWillUnmount: '<',
+    onRender: '<',
+    values: '<'
+  }
+  template = `<test-angular-eight
+                on-change="$ctrl.onChange"
+                on-component-will-unmount="$ctrl.onComponentWillUnmount"
+                on-render="$ctrl.onRender"
+                values="$ctrl.values">
+              </test-angular-eight>`
+  controller = class implements IController {
+    values!: string[]
+
+    constructor(
+      private $scope: IScope
+    ){}
+
+    onChange = (values: string[]) => {
+      this.values = values
+      this.$scope.$apply()
+    }
+  }
+}
+
 const TestAngularOne = react2angular(TestOne, ['foo', 'bar', 'baz'])
 const TestAngularTwo = react2angular(TestTwo, ['foo', 'bar', 'baz'])
 const TestAngularThree = react2angular(TestThree)
 const TestAngularFour = react2angular(TestFour)
 const TestAngularSix = react2angular(TestSix, ['foo'], ['$http', '$element', 'testSixService', 'foo'])
 const TestAngularSeven = react2angular(TestSeven, null, ['foo'])
+const TestAngularEight = react2angular(TestEight, ['values', 'onComponentWillUnmount', 'onRender', 'onChange'])
 
 module('test', ['bcherny/ngimport'])
   .component('testAngularOne', TestAngularOne)
@@ -118,6 +175,8 @@ module('test', ['bcherny/ngimport'])
   .constant('foo', 'CONSTANT FOO')
   .component('testAngularSix', TestAngularSix)
   .component('testAngularSeven', TestAngularSeven)
+  .component('testAngularEight', TestAngularEight)
+  .component('testAngularEightWrapper', new TestEightWrapper())
 
 bootstrap($(), ['test'], { strictDi: true })
 
@@ -355,6 +414,40 @@ describe('react2angular', () => {
       expect(element.find('span').length).toBe(0)
     })
 
-  })
+    it('should not call render after component unmount', () => {
+      const componentWillUnmountSpy = jasmine.createSpy('componentWillUnmount')
+      const renderSpy = jasmine.createSpy('render')
 
+      const scope = Object.assign($rootScope.$new(true), {
+        onComponentWillUnmount: componentWillUnmountSpy,
+        onRender: renderSpy,
+        values: ['val1']
+      })
+      const element = $(`
+        <test-angular-eight-wrapper
+          on-render="onRender"
+          on-component-will-unmount="onComponentWillUnmount"
+          values="values">
+        </test-angular-eight-wrapper>
+      `)
+
+      $compile(element)(scope)
+
+      const childScope = angular
+          .element(element.find('test-angular-eight'))
+          .scope()
+      $rootScope.$apply()
+
+      // Erase first render caused on apply
+      renderSpy.calls.reset()
+
+      // Destroy child component to cause unmount
+      childScope.$destroy()
+
+      // Make sure render on child was not called after unmount
+      expect(componentWillUnmountSpy.calls.count()).toEqual(1)
+      expect(renderSpy.calls.count()).toEqual(0)
+      expect(componentWillUnmountSpy).not.toHaveBeenCalledBefore(renderSpy)
+    })
+  })
 })

tsconfig.json

@@ -4,6 +4,7 @@
     "declaration": false,
     "forceConsistentCasingInFileNames": true,
     "jsx": "react",
+    "skipLibCheck": true,
     "lib": [
       "dom",
       "es2015",
@@ -22,5 +23,8 @@
   "files": [
     "index.tsx",
     "test.tsx"
+  ],
+  "exclude": [
+    "node_modules"
   ]
 }