search

whitphx / react-ymd-date-select

Hooks and components for Y-M-D dropdowns with React
https://whitphx.github.io/react-ymd-date-select/
MIT License
45 stars 6 forks source link

React 18 Support #82

Closed rike422 closed 1 year ago

rike422 commented 1 year ago

Hi. @whitphx

Thank you for releasing this wonderful OSS.

Currently, the package.json configuration supports up to React17, but do you have any plans to support React18?

If I work on an update and make a pull request, will you accept it?

whitphx commented 1 year ago

Thank you for making this issue. Sorry I missed the maintenance time.

If I work on an update and make a pull request, will you accept it?

Yes. PRs are super welcome.

rike422 commented 1 year ago

Thanks for the quick reply @whitphx !

It may be a known issue, but with the current test code, the test seems to fail on react16/react17.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  src/use-date-select.test.ts [ src/use-date-select.test.ts ]
Error: Cannot find module 'react-dom/client'
Require stack:
- /home/akira/dev/github.com/rike422/react-ymd-date-select/node_modules/@testing-library/react/dist/pure.js
 ❯ Object.<anonymous> node_modules/@testing-library/react/dist/pure.js:35:46

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

Test Files  1 failed | 2 passed (3)
     Tests  36 passed (36)
      Time  22ms

It seems that the source code itself is not the problem, but the code under the 'playground or sample directory or in the test environment is causing the error.

The current devDependencies defines the version of react as 18, so I would like to change the peerDependencies to 18 and update "@types/react".

If this library needs to support previous versions, I would like to modify the test environment with the following article. What do you think?

https://medium.com/welldone-software/two-ways-to-run-tests-on-different-versions-of-the-same-library-f-e-react-17-react-16-afb7f861d1e9

rike422 commented 1 year ago

I needed it for my project, so I made the change and sent a pull request. I will add a commit if this library needs follow-up on older versions.

whitphx commented 1 year ago

Thank you, as I have written at the PR, I would like to keep the older versions support.

So, what if setting the React version of devDependencies as 18, but keep the peerDependencies? With this change, I think we don't have to take care about React16 and 17 for testing; It will lack the test coverage with React16 and 17, but I think it's ok for now for simplicity sake.

Of course, if you can pay more efforts on the way you suggested below, it's super welcome and I appreciate it.

If this library needs to support previous versions, I would like to modify the test environment with the following article. What do you think? https://medium.com/welldone-software/two-ways-to-run-tests-on-different-versions-of-the-same-library-f-e-react-17-react-16-afb7f861d1e9

rike422 commented 1 year ago

@whitphx I commented on the PR, I thought you were right, so I fixed it.

As for the version-by-version testing, let me exclude that at this time, as it would be time consuming.

whitphx commented 1 year ago

@rike422 Hi, I released v0.2.0 that ships with your code for the React 18 support.

Please check it out, and re-open this issue if something is broken.

Thank you for your great contribution!