Added typescript and update old dependencies to React v.18 #237
Conversation
fixed re-export types
oleg-yasenytsky
force-pushed
the
master
branch
2 times, most recently
from
7cb2f5a to
794c36d
Compare
|
Thanks alot for doing this <3 |
|
Any chance this could be reviewed and merged in? |
|
@seveibar any chance you can look at this? |
|
Hey guys, taking a look |
| @@ -0,0 +1,7 @@ | |||
| # Default ignored files | |||
There was a problem hiding this comment.
let's remove all the .idea files and add the .idea directory to .gitignore
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "semi": false | |||
| "semi": true | |||
There was a problem hiding this comment.
I think I accidentally changed it, i will fix it
| @@ -1,12 +1,15 @@ | |||
| # React Image Annotate | |||
|
|
|||
| [](https://badge.fury.io/js/react-image-annotate) | |||
| [](https://www.npmjs.com/package/@idapgroup/react-image-annotate) | |||
There was a problem hiding this comment.
we can basically remove the README changes here
| "start": "react-scripts start", | ||
| "test": "react-scripts test", | ||
| "eject": "react-scripts eject", | ||
| "storybook": "start-storybook -p 9090 -s public", |
There was a problem hiding this comment.
hmm removing storybook means removing a lot of tests, is there any way to keep the stories and upgrade storybook? I'm worried that we've basically lost all of our tests.
If storybook isn't your vibe, we could use react cosmos or ladle, but I think keeping the stories (which are essentially tests) will help me make sure that nothing broken.
There was a problem hiding this comment.
i don't have time for update story book and check tests(
There was a problem hiding this comment.
maybe in future i will update storybook too
There was a problem hiding this comment.
I'm generally supportive of migrating this project to Typescript, I exclusively use Typescript now, also supportive of adding people as maintainers to this project since I don't actively use it.
It's not really possible for me to test these changes because the "stories" code, has been removed. The stories are essentially tests
|
Would it be possible to toss this in a branch as-is and add a note in the README saying experimental React 18 support is a work in progress? |
|
If anyone need package with this pull, i deploy it npm.js |
I'm eager to merge this, but I think my comments are pretty reasonable, for a project this old its good to make sure you don't lose testing surface area, and some of the other changes would misdirect the user, eg the README wouldn't link to user to the npm package published by this repo. To clarify: I'm very eager to merge and make people maintainers |
No description provided.