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

Upgrade to React 18 #1

Merged
merged 7 commits into from
Feb 19, 2024
Merged

Upgrade to React 18 #1

merged 7 commits into from
Feb 19, 2024

Conversation

aarr0n
Copy link
Contributor

@aarr0n aarr0n commented Jan 18, 2024

Upgrading to React18 and bumping dependencies as well.

@@ -1,4 +1,5 @@
module.exports = {
preset: 'ts-jest',
testEnvironment: 'jsdom',
testRunner: 'jest-jasmine2',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -14,6 +14,8 @@ import * as React from 'react';
import { Simulate } from 'react-dom/test-utils';
import { reactular } from './reactular';

const wait = () => new Promise(resolve => setTimeout(resolve, 10));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an alternative to using act which I couldn't use because Jest thinks we're not in a React environment.

@aarr0n aarr0n requested a review from edsrzf February 12, 2024 19:57
Copy link
Contributor

@edsrzf edsrzf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few small suggestions

"react": "^16.9.0 || ^17.0.0",
"react-dom": "^16.9.0 || ^17.0.0"
"@types/react": "^18.0.0",
"react": "^18.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal at all, but would these changes be compatible with React 16 or 17? I'm guessing not, but just double-checking.

I'm not sure if anyone else actually uses this package, but it could be nice to allow older versions if it's not a burden.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Root API is React 18 only unfortunately. If we leave things as they are and use render, React will just behave as if its running React 17.

reactular.tsx Outdated Show resolved Hide resolved
reactular.tsx Outdated Show resolved Hide resolved
@aarr0n aarr0n merged commit 2ed6f7e into main Feb 19, 2024
6 checks passed
@aarr0n aarr0n deleted the ap--upgrade-to-react18 branch February 19, 2024 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants