Convert ES6/TypeScript/CoffeeScript Tests to createRoot + act (facebo…

…ok#21598) * Convert ES6/TypeScript CoffeeScript Tests to createRoot + act * Change expectation for WWW+VARIANT because the deferRenderPhaseUpdateToNextBatch flag breaks this behavior
koto · Jun 15, 2021 · 3d2bd50 · 3d2bd50
1 parent 127f2a6
commit 3d2bd50
Show file tree

Hide file tree

Showing 7 changed files with 152 additions and 113 deletions.
diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
@@ -5,21 +5,25 @@ This source code is licensed under the MIT license found in the
 LICENSE file in the root directory of this source tree.
 ###
 
+PropTypes = null
 React = null
 ReactDOM = null
-PropTypes = null
+act = null
 
 describe 'ReactCoffeeScriptClass', ->
   container = null
+  root = null
   InnerComponent = null
   attachedListener = null;
   renderedName = null;
 
   beforeEach ->
     React = require 'react'
     ReactDOM = require 'react-dom'
+    act = require('react-dom/test-utils').act
     PropTypes = require 'prop-types'
     container = document.createElement 'div'
+    root = ReactDOM.createRoot container
     attachedListener = null
     renderedName = null
     InnerComponent = class extends React.Component
@@ -30,11 +34,11 @@ describe 'ReactCoffeeScriptClass', ->
         return React.createElement('div', className: this.props.name)
 
   test = (element, expectedTag, expectedClassName) ->
-    instance = ReactDOM.render(element, container)
+    act ->
+      root.render(element)
     expect(container.firstChild).not.toBeNull()
     expect(container.firstChild.tagName).toBe(expectedTag)
     expect(container.firstChild.className).toBe(expectedClassName)
-    instance;
 
   it 'preserves the name of the class for use in error messages', ->
     class Foo extends React.Component
@@ -44,14 +48,16 @@ describe 'ReactCoffeeScriptClass', ->
     class Foo extends React.Component
     expect(->
       expect(->
-        ReactDOM.render React.createElement(Foo), container
+        act ->
+          root.render React.createElement(Foo)
       ).toThrow()
     ).toErrorDev([
-      # A failed component renders twice in DEV
+      # A failed component renders four times in DEV in concurrent mode
+      'No `render` method found on the returned component instance',
+      'No `render` method found on the returned component instance',
       'No `render` method found on the returned component instance',
       'No `render` method found on the returned component instance',
     ])
-    undefined
 
   it 'renders a simple stateless component with prop', ->
     class Foo extends React.Component
@@ -62,7 +68,6 @@ describe 'ReactCoffeeScriptClass', ->
 
     test React.createElement(Foo, bar: 'foo'), 'DIV', 'foo'
     test React.createElement(Foo, bar: 'bar'), 'DIV', 'bar'
-    undefined
 
   it 'renders based on state using initial values in this.props', ->
     class Foo extends React.Component
@@ -76,7 +81,6 @@ describe 'ReactCoffeeScriptClass', ->
         )
 
     test React.createElement(Foo, initialValue: 'foo'), 'SPAN', 'foo'
-    undefined
 
   it 'renders based on state using props in the constructor', ->
     class Foo extends React.Component
@@ -95,10 +99,10 @@ describe 'ReactCoffeeScriptClass', ->
           className: @state.bar
         )
 
-    instance = test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo'
-    instance.changeState()
+    ref = React.createRef()
+    test React.createElement(Foo, initialValue: 'foo', ref: ref), 'DIV', 'foo'
+    ref.current.changeState()
     test React.createElement(Foo), 'SPAN', 'bar'
-    undefined
 
   it 'sets initial state with value returned by static getDerivedStateFromProps', ->
     class Foo extends React.Component
@@ -115,7 +119,6 @@ describe 'ReactCoffeeScriptClass', ->
         bar: 'bar'
       }
     test React.createElement(Foo, foo: 'foo'), 'DIV', 'foo bar'
-    undefined
 
   it 'warns if getDerivedStateFromProps is not static', ->
     class Foo extends React.Component
@@ -124,9 +127,9 @@ describe 'ReactCoffeeScriptClass', ->
       getDerivedStateFromProps: ->
         {}
     expect(->
-      ReactDOM.render(React.createElement(Foo, foo: 'foo'), container)
+      act ->
+        root.render React.createElement(Foo, foo: 'foo')
     ).toErrorDev 'Foo: getDerivedStateFromProps() is defined as an instance method and will be ignored. Instead, declare it as a static method.'
-    undefined
 
   it 'warns if getDerivedStateFromError is not static', ->
     class Foo extends React.Component
@@ -135,9 +138,9 @@ describe 'ReactCoffeeScriptClass', ->
       getDerivedStateFromError: ->
         {}
     expect(->
-      ReactDOM.render(React.createElement(Foo, foo: 'foo'), container)
+      act ->
+        root.render React.createElement(Foo, foo: 'foo')
     ).toErrorDev 'Foo: getDerivedStateFromError() is defined as an instance method and will be ignored. Instead, declare it as a static method.'
-    undefined
 
   it 'warns if getSnapshotBeforeUpdate is static', ->
     class Foo extends React.Component
@@ -146,9 +149,9 @@ describe 'ReactCoffeeScriptClass', ->
     Foo.getSnapshotBeforeUpdate = () ->
       {}
     expect(->
-      ReactDOM.render(React.createElement(Foo, foo: 'foo'), container)
+      act ->
+        root.render React.createElement(Foo, foo: 'foo')
     ).toErrorDev 'Foo: getSnapshotBeforeUpdate() is defined as a static method and will be ignored. Instead, declare it as an instance method.'
-    undefined
 
   it 'warns if state not initialized before static getDerivedStateFromProps', ->
     class Foo extends React.Component
@@ -162,14 +165,14 @@ describe 'ReactCoffeeScriptClass', ->
         bar: 'bar'
       }
     expect(->
-      ReactDOM.render(React.createElement(Foo, foo: 'foo'), container)
+      act ->
+        root.render React.createElement(Foo, foo: 'foo')
     ).toErrorDev (
       '`Foo` uses `getDerivedStateFromProps` but its initial state is ' +
       'undefined. This is not recommended. Instead, define the initial state by ' +
       'assigning an object to `this.state` in the constructor of `Foo`. ' +
       'This ensures that `getDerivedStateFromProps` arguments have a consistent shape.'
     )
-    undefined
 
   it 'updates initial state with values returned by static getDerivedStateFromProps', ->
     class Foo extends React.Component
@@ -187,7 +190,6 @@ describe 'ReactCoffeeScriptClass', ->
         foo: "not-#{prevState.foo}"
       }
     test React.createElement(Foo), 'DIV', 'not-foo bar'
-    undefined
 
   it 'renders updated state with values returned by static getDerivedStateFromProps', ->
     class Foo extends React.Component
@@ -207,7 +209,6 @@ describe 'ReactCoffeeScriptClass', ->
       return null
     test React.createElement(Foo, update: false), 'DIV', 'initial'
     test React.createElement(Foo, update: true), 'DIV', 'updated'
-    undefined
 
   it 'renders based on context in the constructor', ->
     class Foo extends React.Component
@@ -239,7 +240,6 @@ describe 'ReactCoffeeScriptClass', ->
         React.createElement Foo
 
     test React.createElement(Outer), 'SPAN', 'foo'
-    undefined
 
   it 'renders only once when setting state in componentWillMount', ->
     renderCount = 0
@@ -255,8 +255,9 @@ describe 'ReactCoffeeScriptClass', ->
         React.createElement('span', className: @state.bar)
 
     test React.createElement(Foo, initialValue: 'foo'), 'SPAN', 'bar'
-    expect(renderCount).toBe 1
-    undefined
+    # This is broken with deferRenderPhaseUpdateToNextBatch flag on.
+    # We can't use the gate feature here because this test is also in CoffeeScript and TypeScript.
+    expect(renderCount).toBe(if global.__WWW__ and !global.__VARIANT__ then 2 else 1)
 
   it 'should warn with non-object in the initial state property', ->
     [['an array'], 'a string', 1234].forEach (state) ->
@@ -270,7 +271,6 @@ describe 'ReactCoffeeScriptClass', ->
       expect(->
         test React.createElement(Foo), 'SPAN', ''
       ).toErrorDev('Foo.state: must be set to an object or null')
-    undefined
 
   it 'should render with null in the initial state property', ->
     class Foo extends React.Component
@@ -281,7 +281,6 @@ describe 'ReactCoffeeScriptClass', ->
         React.createElement('span')
 
     test React.createElement(Foo), 'SPAN', ''
-    undefined
 
   it 'setState through an event handler', ->
     class Foo extends React.Component
@@ -298,9 +297,9 @@ describe 'ReactCoffeeScriptClass', ->
         )
 
     test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo'
-    attachedListener()
+    act ->
+      attachedListener()
     expect(renderedName).toBe 'bar'
-    undefined
 
   it 'should not implicitly bind event handlers', ->
     class Foo extends React.Component
@@ -318,7 +317,6 @@ describe 'ReactCoffeeScriptClass', ->
 
     test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo'
     expect(attachedListener).toThrow()
-    undefined
 
   it 'renders using forceUpdate even when there is no state', ->
     class Foo extends React.Component
@@ -336,9 +334,9 @@ describe 'ReactCoffeeScriptClass', ->
         )
 
     test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo'
-    attachedListener()
+    act ->
+      attachedListener()
     expect(renderedName).toBe 'bar'
-    undefined
 
   it 'will call all the normal life cycle methods', ->
     lifeCycles = []
@@ -387,9 +385,9 @@ describe 'ReactCoffeeScriptClass', ->
       'did-update',    { value: 'foo' }, {}
     ]
     lifeCycles = [] # reset
-    ReactDOM.unmountComponentAtNode container
+    act ->
+      root.unmount()
     expect(lifeCycles).toEqual ['will-unmount']
-    undefined
 
   it 'warns when classic properties are defined on the instance,
       but does not invoke them.', ->
@@ -425,7 +423,6 @@ describe 'ReactCoffeeScriptClass', ->
     ])
     expect(getInitialStateWasCalled).toBe false
     expect(getDefaultPropsWasCalled).toBe false
-    undefined
 
   it 'does not warn about getInitialState() on class components
       if state is also defined.', ->
@@ -443,7 +440,6 @@ describe 'ReactCoffeeScriptClass', ->
         )
 
     test React.createElement(Foo), 'SPAN', 'foo'
-    undefined
 
   it 'should warn when misspelling shouldComponentUpdate', ->
     class NamedComponent extends React.Component
@@ -462,7 +458,6 @@ describe 'ReactCoffeeScriptClass', ->
        Did you mean shouldComponentUpdate()? The name is phrased as a
        question because the function is expected to return a value.'
     )
-    undefined
 
   it 'should warn when misspelling componentWillReceiveProps', ->
     class NamedComponent extends React.Component
@@ -480,7 +475,6 @@ describe 'ReactCoffeeScriptClass', ->
       'Warning: NamedComponent has a method called componentWillRecieveProps().
        Did you mean componentWillReceiveProps()?'
     )
-    undefined
 
   it 'should warn when misspelling UNSAFE_componentWillReceiveProps', ->
     class NamedComponent extends React.Component
@@ -498,24 +492,22 @@ describe 'ReactCoffeeScriptClass', ->
       'Warning: NamedComponent has a method called UNSAFE_componentWillRecieveProps().
        Did you mean UNSAFE_componentWillReceiveProps()?'
     )
-    undefined
 
   it 'should throw AND warn when trying to access classic APIs', ->
-    instance =
-      test React.createElement(InnerComponent, name: 'foo'), 'DIV', 'foo'
+    ref = React.createRef()
+    test React.createElement(InnerComponent, name: 'foo', ref: ref), 'DIV', 'foo'
     expect(->
-      expect(-> instance.replaceState {}).toThrow()
+      expect(-> ref.current.replaceState {}).toThrow()
     ).toWarnDev(
       'replaceState(...) is deprecated in plain JavaScript React classes',
       {withoutStack: true}
     )
     expect(->
-      expect(-> instance.isMounted()).toThrow()
+      expect(-> ref.current.isMounted()).toThrow()
     ).toWarnDev(
       'isMounted(...) is deprecated in plain JavaScript React classes',
       {withoutStack: true}
     )
-    undefined
 
   it 'supports this.context passed via getChildContext', ->
     class Bar extends React.Component
@@ -533,7 +525,6 @@ describe 'ReactCoffeeScriptClass', ->
         React.createElement Bar
 
     test React.createElement(Foo), 'DIV', 'bar-through-context'
-    undefined
 
   it 'supports classic refs', ->
     class Foo extends React.Component
@@ -543,13 +534,14 @@ describe 'ReactCoffeeScriptClass', ->
           ref: 'inner'
         )
 
-    instance = test(React.createElement(Foo), 'DIV', 'foo')
-    expect(instance.refs.inner.getName()).toBe 'foo'
-    undefined
+    ref = React.createRef()
+    test(React.createElement(Foo, ref: ref), 'DIV', 'foo')
+    expect(ref.current.refs.inner.getName()).toBe 'foo'
 
   it 'supports drilling through to the DOM using findDOMNode', ->
-    instance = test React.createElement(InnerComponent, name: 'foo'), 'DIV', 'foo'
-    node = ReactDOM.findDOMNode(instance)
+    ref = React.createRef()
+    test React.createElement(InnerComponent, name: 'foo', ref: ref), 'DIV', 'foo'
+    node = ReactDOM.findDOMNode(ref.current)
     expect(node).toBe container.firstChild
-    undefined
+
   undefined