Skip to content

Commit

Permalink
Support React 18 strict mode (#100)
Browse files Browse the repository at this point in the history
* Support React 18 strict mode

React 18 changes how strict mode works in DEV - When a component is mounted, React now unmounts it then immediately mounts it again with the previous state.
This surfaced a crash that mostly happened during hot refresh.

* Upgrade to Jest 29

* Fix warnings when running tests after upgrading to React 18

* Don't run on Node 12
  • Loading branch information
guyca committed Jun 24, 2023
1 parent 52979b0 commit c481353
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 28 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:

strategy:
matrix:
node-version: [12.x, 14.x]
node-version: [14.x]

env:
NPM_EMAIL: ''
Expand Down
17 changes: 9 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@
"@babel/preset-typescript": "7.18.x",
"@babel/types": "7.20.x",
"@johanblumenberg/ts-mockito": "1.x.x",
"@testing-library/react": "^12.1.2",
"@testing-library/react": "14.x.x",
"@testing-library/react-hooks": "^7.0.2",
"@types/hoist-non-react-statics": "^3.3.1",
"@types/jest": "26.x.x",
"@types/jest": "29.5.x",
"@types/lodash": "^4.14.176",
"@types/react": "^17.0.34",
"@types/react-dom": "^17.0.11",
"@types/react": "18.2.x",
"@types/react-dom": "18.2.x",
"@types/react-test-renderer": "17.0.1",
"@typescript-eslint/eslint-plugin": "^5.3.0",
"@typescript-eslint/parser": "^5.3.0",
Expand All @@ -46,10 +46,11 @@
"eslint-plugin-react": "^7.26.1",
"eslint-plugin-react-hooks": "^4.2.0",
"eslint-plugin-unused-imports": "2.x.x",
"jest": "27.x.x",
"jest": "29.5.x",
"jest-environment-jsdom": "^29.5.0",
"lodash": "^4.17.21",
"react": "17.0.2",
"react-dom": "17.0.2",
"react": "18.2.x",
"react-dom": "18.2.x",
"setimmediate": "^1.0.5",
"typescript": "^4.5.4"
},
Expand Down Expand Up @@ -78,4 +79,4 @@
"url": "https://github.com/wix-incubator/react-obsidian/issues"
},
"homepage": "https://github.com/wix-incubator/react-obsidian#readme"
}
}
4 changes: 2 additions & 2 deletions src/graph/registry/GraphRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ export class GraphRegistry {
}

clear(graph: Graph) {
const Graph = this.instanceToConstructor.get(graph)!;
if (this.isSingleton(Graph)) return;
const Graph = this.instanceToConstructor.get(graph);
if (!Graph || this.isSingleton(Graph)) return;
this.instanceToConstructor.delete(graph);
this.constructorToInstance.get(Graph)!.delete(graph);
this.nameToInstance.delete(graph.name);
Expand Down
2 changes: 1 addition & 1 deletion src/injectors/hooks/HookInjector.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { renderHook } from '@testing-library/react-hooks';
import { renderHook } from '@testing-library/react';
import { useState } from 'react';
import StringProvider from '../../../test/fixtures/StringProvider';
import Subgraph from '../../../test/fixtures/Subgraph';
Expand Down
2 changes: 1 addition & 1 deletion src/injectors/hooks/InjectHook.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { renderHook } from '@testing-library/react-hooks';
import { renderHook } from '@testing-library/react';
import MainGraph from '../../../test/fixtures/MainGraph';
import Subgraph from '../../../test/fixtures/Subgraph';
import { DependenciesOf } from '../../types';
Expand Down
2 changes: 1 addition & 1 deletion src/observable/useObserver.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { act, renderHook } from '@testing-library/react-hooks';
import { act, renderHook } from '@testing-library/react';
import _ from 'lodash';
import { Observable } from './Observable';
import { useObserver } from './useObserver';
Expand Down
19 changes: 19 additions & 0 deletions test/integration/reactStrictMode.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React from 'react';
import { render } from '@testing-library/react';
import { DependenciesOf, injectComponent } from '../../src';
import { LifecycleBoundGraph } from '../fixtures/LifecycleBoundGraph';

describe('React Strict Mode', () => {
it('should render without crashing', () => {
const { container } = render(<InjectedComponent stringFromProps={'foo'}/>, {
wrapper: React.StrictMode,
});
expect(container.textContent).toBe('A string passed via props: foo');
});
});

const Component = ({ computedFromProps }: DependenciesOf<LifecycleBoundGraph, 'computedFromProps'>) => {
return <>{computedFromProps}</>;
};

const InjectedComponent = injectComponent<{ stringFromProps: string }>(Component, LifecycleBoundGraph);
28 changes: 14 additions & 14 deletions transformers/babel-plugin-obsidian/__snapshots__/index.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,24 @@ exports[`Provider Arguments Transformer Adds method name to provider arguments (
"var _dec, _class;
function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }
let MainGraph = (_dec = Provides({
name: \\"someString\\"
name: "someString"
}), (_class = class MainGraph {
someString({
stringProvider: stringProvider,
emptyString: emptyString
}) {
return stringProvider.theString + emptyString;
}
}, (_applyDecoratedDescriptor(_class.prototype, \\"someString\\", [_dec], Object.getOwnPropertyDescriptor(_class.prototype, \\"someString\\"), _class.prototype)), _class));"
}, (_applyDecoratedDescriptor(_class.prototype, "someString", [_dec], Object.getOwnPropertyDescriptor(_class.prototype, "someString"), _class.prototype)), _class));"
`;

exports[`Provider Arguments Transformer Adds property name to @Inject arguments @Inject -> @Inject("myDependency") 1`] = `
"var _dec, _class, _descriptor;
function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }
function _initializerWarningHelper(descriptor, context) { throw new Error('Decorating class property failed. Please ensure that ' + 'transform-class-properties is enabled and runs after the decorators transform.'); }
let MainGraph = (_dec = Inject(\\"someString\\"), (_class = class MainGraph {
let MainGraph = (_dec = Inject("someString"), (_class = class MainGraph {
someString = _initializerWarningHelper(_descriptor, this);
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, \\"someString\\", [_dec], {
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, "someString", [_dec], {
configurable: true,
enumerable: true,
writable: true,
Expand All @@ -33,9 +33,9 @@ exports[`Provider Arguments Transformer Adds property name to @LateInject argume
"var _dec, _class, _descriptor;
function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }
function _initializerWarningHelper(descriptor, context) { throw new Error('Decorating class property failed. Please ensure that ' + 'transform-class-properties is enabled and runs after the decorators transform.'); }
let MainGraph = (_dec = LateInject(\\"someString\\"), (_class = class MainGraph {
let MainGraph = (_dec = LateInject("someString"), (_class = class MainGraph {
someString = _initializerWarningHelper(_descriptor, this);
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, \\"someString\\", [_dec], {
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, "someString", [_dec], {
configurable: true,
enumerable: true,
writable: true,
Expand All @@ -54,16 +54,16 @@ let MainGraph = (_dec = Provides({
}) {
return stringProvider.theString;
}
}, (_applyDecoratedDescriptor(_class.prototype, \\"someString\\", [_dec], Object.getOwnPropertyDescriptor(_class.prototype, \\"someString\\"), _class.prototype)), _class));"
}, (_applyDecoratedDescriptor(_class.prototype, "someString", [_dec], Object.getOwnPropertyDescriptor(_class.prototype, "someString"), _class.prototype)), _class));"
`;

exports[`Provider Arguments Transformer Does not add property name to @Inject if name is provided by the user 1`] = `
"var _dec, _class, _descriptor;
function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }
function _initializerWarningHelper(descriptor, context) { throw new Error('Decorating class property failed. Please ensure that ' + 'transform-class-properties is enabled and runs after the decorators transform.'); }
let MainGraph = (_dec = Inject(\\"someString\\"), (_class = class MainGraph {
let MainGraph = (_dec = Inject("someString"), (_class = class MainGraph {
someString = _initializerWarningHelper(_descriptor, this);
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, \\"someString\\", [_dec], {
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, "someString", [_dec], {
configurable: true,
enumerable: true,
writable: true,
Expand All @@ -75,9 +75,9 @@ exports[`Provider Arguments Transformer Does not add property name to @LateInjec
"var _dec, _class, _descriptor;
function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }
function _initializerWarningHelper(descriptor, context) { throw new Error('Decorating class property failed. Please ensure that ' + 'transform-class-properties is enabled and runs after the decorators transform.'); }
let MainGraph = (_dec = LateInject(\\"someString\\"), (_class = class MainGraph {
let MainGraph = (_dec = LateInject("someString"), (_class = class MainGraph {
someString = _initializerWarningHelper(_descriptor, this);
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, \\"someString\\", [_dec], {
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, "someString", [_dec], {
configurable: true,
enumerable: true,
writable: true,
Expand All @@ -89,17 +89,17 @@ exports[`Provider Arguments Transformer handles providers that have no arguments
"var _dec, _class;
function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }
let MainGraph = (_dec = Provides({
name: \\"someString\\"
name: "someString"
}), (_class = class MainGraph {
someString() {
return 'someString';
}
}, (_applyDecoratedDescriptor(_class.prototype, \\"someString\\", [_dec], Object.getOwnPropertyDescriptor(_class.prototype, \\"someString\\"), _class.prototype)), _class));"
}, (_applyDecoratedDescriptor(_class.prototype, "someString", [_dec], Object.getOwnPropertyDescriptor(_class.prototype, "someString"), _class.prototype)), _class));"
`;

exports[`Provider Arguments Transformer saves constructor argument name in Inject - @Inject -> @Inject(arg) 1`] = `
"class MainGraph {
constructor(@Inject(\\"arg\\")
constructor(@Inject("arg")
arg) {}
}"
`;

0 comments on commit c481353

Please sign in to comment.