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

React18 support #3387

Merged
merged 12 commits into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/swift-ravens-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"mobx-react": minor
"mobx-react-lite": minor
---

Added experimental support for React 18. Fixes #3363, #2526. Supersedes #3005
1 change: 1 addition & 0 deletions jest.base.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module.exports = function buildConfig(packageDirectory, pkgConfig) {
const packageTsconfig = path.resolve(packageDirectory, tsConfig)
return {
preset: "ts-jest/presets/js-with-ts",
testEnvironment: "jest-environment-jsdom-fifteen",
globals: {
__DEV__: true,
"ts-jest": {
Expand Down
17 changes: 9 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@
"devDependencies": {
"@changesets/changelog-github": "^0.2.7",
"@changesets/cli": "^2.11.0",
"@testing-library/jest-dom": "^5.1.1",
"@testing-library/react": "^11.1.1",
"@testing-library/react-hooks": "3.4.2",
"@testing-library/jest-dom": "^5.16.4",
"@testing-library/react": "^13.0.0",
"@testing-library/react-hooks": "7.0.2",
"@types/jest": "^26.0.15",
"@types/node": "14",
"@types/prop-types": "^15.5.2",
"@types/react": "^16.8.24",
"@types/react-dom": "^16.0.5",
"@types/react": "^18.0.0",
"@types/react-dom": "^18.0.0",
"@typescript-eslint/eslint-plugin": "^4.6.1",
"@typescript-eslint/parser": "^4.1.1",
"coveralls": "^3.1.0",
Expand All @@ -49,6 +49,7 @@
"import-size": "^1.0.2",
"iterall": "^1.3.0",
"jest": "^26.6.2",
"jest-environment-jsdom-fifteen": "^1.0.2",
"jest-mock-console": "^1.0.1",
"lerna": "^3.22.1",
"lint-staged": "^10.1.7",
Expand All @@ -58,9 +59,9 @@
"prettier": "^2.0.5",
"pretty-quick": "3.1.0",
"prop-types": "15.6.2",
"react": "^17.0.0",
"react-dom": "^17.0.0",
"react-test-renderer": "^17.0.0",
"react": "^18.0.0",
"react-dom": "^18.0.0",
"react-test-renderer": "^18.0.0",
"serializr": "^2.0.3",
"tape": "^5.0.1",
"ts-jest": "26.4.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ exports[`issue 12 init state is correct 1`] = `
</span>
<span>
tea

</span>
</div>
</div>
Expand All @@ -40,7 +39,6 @@ exports[`issue 12 init state is correct 2`] = `
</span>
<span>
tea

</span>
</div>
</div>
Expand All @@ -51,7 +49,6 @@ exports[`issue 12 run transaction 1`] = `
<div>
<span>
soup

</span>
</div>
</div>
Expand All @@ -62,7 +59,6 @@ exports[`issue 12 run transaction 2`] = `
<div>
<span>
soup

</span>
</div>
</div>
Expand Down
26 changes: 16 additions & 10 deletions packages/mobx-react-lite/__tests__/observer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ function runTestSuite(mode: "observer" | "useObserver") {
const x = mobx.observable.box(1)
const errorsSeen: any[] = []

class ErrorBoundary extends React.Component {
class ErrorBoundary extends React.Component<{ children: any }> {
public static getDerivedStateFromError() {
return { hasError: true }
}
Expand Down Expand Up @@ -488,9 +488,10 @@ test("observer(cmp, { forwardRef: true }) + useImperativeHandle", () => {

interface IProps {
value: string
ref: React.Ref<IMethods>
}

const FancyInput = observer(
const FancyInput = observer<IProps>(
(props: IProps, ref: React.Ref<IMethods>) => {
const inputRef = React.useRef<HTMLInputElement>(null)
React.useImperativeHandle(
Expand Down Expand Up @@ -673,15 +674,15 @@ test("parent / childs render in the right order", done => {
render(<Parent />)

tryLogout()
expect(events).toEqual(["parent", "child", "parent"])
expect(events).toEqual(["parent", "child"])
done()
})

it("should have overload for props with children", () => {
interface IProps {
value: string
}
const TestComponent = observer<IProps>(({ value, children }) => {
const TestComponent = observer<IProps>(({ value }) => {
return null
})

Expand All @@ -697,7 +698,7 @@ it("should have overload for empty options", () => {
interface IProps {
value: string
}
const TestComponent = observer<IProps>(({ value, children }) => {
const TestComponent = observer<IProps>(({ value }) => {
return null
}, {})

Expand All @@ -715,7 +716,7 @@ it("should have overload for props with children when forwardRef", () => {
value: string
}
const TestComponent = observer<IProps, IMethods>(
({ value, children }, ref) => {
({ value }, ref) => {
return null
},
{ forwardRef: true }
Expand Down Expand Up @@ -1032,17 +1033,21 @@ test("Anonymous component displayName #3192", () => {
// React prints errors even if we catch em
const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {})

// Simulate:
// Error: n_a_m_e(...): Nothing was returned from render. This usually means a return statement is missing. Or, to render nothing, return null.
// Simulate returning something not renderable:
// Error: n_a_m_e(...):
// The point is to get correct displayName in error msg.

let memoError
let observerError

// @ts-ignore
const MemoCmp = React.memo(() => {})
const MemoCmp = React.memo(() => {
return { hello: "world" }
})
// @ts-ignore
const ObserverCmp = observer(() => {})
const ObserverCmp = observer(() => {
return { hello: "world" }
})

ObserverCmp.displayName = MemoCmp.displayName = "n_a_m_e"

Expand All @@ -1053,6 +1058,7 @@ test("Anonymous component displayName #3192", () => {
}

try {
// @ts-ignore
render(<ObserverCmp />)
} catch (error) {
observerError = error
Expand Down
33 changes: 14 additions & 19 deletions packages/mobx-react-lite/__tests__/strictAndConcurrentMode.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { act, cleanup, render } from "@testing-library/react"
import mockConsole from "jest-mock-console"
import * as mobx from "mobx"
import * as React from "react"
import ReactDOM from "react-dom"

import { useObserver } from "../src/useObserver"

Expand Down Expand Up @@ -42,36 +41,32 @@ test("uncommitted observing components should not attempt state changes", () =>
}
})

const strictModeValues = [true, false]
const strictModeValues = [true]
kubk marked this conversation as resolved.
Show resolved Hide resolved
strictModeValues.forEach(strictMode => {
const modeName = strictMode ? "StrictMode" : "non-StrictMode"

test(`observable changes before first commit are not lost (${modeName})`, () => {
test(`observable changes before first commit are not lost (${modeName})`, async () => {
const store = mobx.observable({ value: "initial" })

const TestComponent = () => useObserver(() => <div>{store.value}</div>)
const TestComponent = () =>
useObserver(() => {
const res = <div>{store.value}</div>
// Change our observable. This is happening between the initial render of
// our component and its initial commit, so it isn't fully mounted yet.
// We want to ensure that the change isn't lost.
store.value = "changed"
return res
})

// Render our observing component wrapped in StrictMode, but using
// raw ReactDOM.render (not react-testing-library render) because we
// don't want the useEffect calls to have run just yet...
const rootNode = document.createElement("div")
document.body.appendChild(rootNode)

let elem = <TestComponent />
if (strictMode) {
elem = <React.StrictMode>{elem}</React.StrictMode>
}
const rendering = render(elem)

ReactDOM.render(elem, rootNode)

// Change our observable. This is happening between the initial render of
// our component and its initial commit, so it isn't fully mounted yet.
// We want to ensure that the change isn't lost.
store.value = "changed"

act(() => {
// no-op
})

expect(rootNode.textContent).toBe("changed")
expect(rendering.baseElement.textContent).toBe("changed")
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import "./utils/killFinalizationRegistry"
import { act, cleanup, render } from "@testing-library/react"
import * as mobx from "mobx"
import * as React from "react"
import ReactDOM from "react-dom"

import { useObserver } from "../src/useObserver"
import {
Expand Down Expand Up @@ -90,19 +89,13 @@ test("cleanup timer should not clean up recently-pended reactions", () => {

const TestComponent1 = () => useObserver(() => <div>{store.count}</div>)

// We're going to render directly using ReactDOM, not react-testing-library, because we want
// to be able to get in and do nasty things before everything (including useEffects) have completed,
// and react-testing-library waits for all that, using act().

const rootNode = document.createElement("div")
ReactDOM.render(
const rendering = render(
// We use StrictMode here, but it would be helpful to switch this to use real
// concurrent mode: we don't have a true async render right now so this test
// isn't as thorough as it could be.
<React.StrictMode>
<TestComponent1 />
</React.StrictMode>,
rootNode
</React.StrictMode>
)

// We need to trigger our cleanup timer to run. We can't do this simply
Expand All @@ -127,7 +120,9 @@ test("cleanup timer should not clean up recently-pended reactions", () => {
expect(countIsObserved).toBeTruthy()
})

test("component should recreate reaction if necessary", () => {
// TODO: MWE: disabled during React 18 migration, not sure how to express it icmw with testing-lib,
// and using new React renderRoot will fail icmw JSDOM
test.skip("component should recreate reaction if necessary", () => {
// There _may_ be very strange cases where the reaction gets tidied up
// but is actually still needed. This _really_ shouldn't happen.
// e.g. if we're using Suspense and the component starts to render,
Expand Down Expand Up @@ -159,15 +154,10 @@ test("component should recreate reaction if necessary", () => {

const TestComponent1 = () => useObserver(() => <div>{store.count}</div>)

// We're going to render directly using ReactDOM, not react-testing-library, because we want
// to be able to get in and do nasty things before everything (including useEffects) have completed,
// and react-testing-library waits for all that, using act().
const rootNode = document.createElement("div")
ReactDOM.render(
const rendering = render(
<React.StrictMode>
<TestComponent1 />
</React.StrictMode>,
rootNode
</React.StrictMode>
)

// We need to trigger our cleanup timer to run. We don't want
Expand Down Expand Up @@ -198,5 +188,5 @@ test("component should recreate reaction if necessary", () => {
// and the component should have rendered enough to
// show the latest value, which was set whilst it
// wasn't even looking.
expect(rootNode.textContent).toContain("42")
expect(rendering.baseElement.textContent).toContain("42")
})
2 changes: 1 addition & 1 deletion packages/mobx-react-lite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"dependencies": {},
"peerDependencies": {
"mobx": "^6.1.0",
"react": "^16.8.0 || ^17"
"react": "^16.8.0 || ^17 || ^18"
},
"peerDependenciesMeta": {
"react-dom": {
Expand Down
2 changes: 1 addition & 1 deletion packages/mobx-react-lite/src/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export function observer<P extends object, TRef = {}>(
}
}

let observerComponent = (props: P, ref: React.Ref<TRef>) => {
let observerComponent = (props: any, ref: React.Ref<TRef>) => {
return useObserver(() => render(props, ref), baseComponentName)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ exports[`issue 12 1`] = `
</span>
<span>
tea

</span>
</div>
</div>
Expand All @@ -20,7 +19,6 @@ exports[`issue 12 2`] = `
<div>
<span>
soup

</span>
</div>
</div>
Expand Down
8 changes: 6 additions & 2 deletions packages/mobx-react/__tests__/hooks.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ test("computed properties react to props when using hooks", async () => {
const { container } = render(<Parent />)
expect(container).toHaveTextContent("0")

jest.runAllTimers()
act(() => {
jest.runAllTimers()
})
expect(seen).toEqual(["parent", 0, "parent", 2])
expect(container).toHaveTextContent("2")
})
Expand Down Expand Up @@ -78,7 +80,9 @@ test("computed properties result in double render when using observer instead of
const { container } = render(<Parent />)
expect(container).toHaveTextContent("0")

jest.runAllTimers()
act(() => {
jest.runAllTimers()
})
expect(seen).toEqual([
"parent",
0,
Expand Down
Loading