Refactor React components to fall more inline with existing React libraries about skeleton HOT 4 OPEN

So, the syntactical changes (other than removing the return types) have literally no performance impact whatsoever.

@Hugos68, I do apologize, I actually misspoke to some extent, I remembered like right before I went to sleep exactly what I was doing wrong when it comes to paths. You are correct in that Vite doesn't do anything with tsconfig files. In fact, per the Vite docs, it really take the tsconfig.json into account. I forgot that Vite has it's own paths parameter. I should have known about that from my limited Svelte development. On the React side, I've always used Webpack (until recently) since that was the standard on the React side for so long, and since you guys are using "moduleResolution": "bundler", Vite would be the correct place to do that anyway. As for the .js extensions. Oh boy, is that a can of worms. I never understood how deep that issue is since I have never encountered it, but there are a ton of issues open in the nodejs/node and microsoft/TypeScript repos for that. I think the best issue to highlight the problem, which actually links to a few others though is nodejs/node#46006. With that said, it probably is best to just leave the .js extensions in place since they don't really hurt anything.

With that said, I guess my plan of action for the time being is to set up the path alias with both Vite and TypeScript (Vite for building, and TypeScript for development) and switch everything over to using the reactCompose helper. Then, I can work on refactoring components to optimize performance. However, I will open a separate issue for all of that and just link the issues back to this. I guess we can use this issue to track the overall progress and use the smaller issues and PRs to track the individual changes.

For anyone interested in what the React best practices look like, there are 3 important things to be familiar with:

• the Rules of Hooks (which is enforced by eslint-plugin-react-hooks)
• the Rules of React (which is enforced by eslint-plugin-react-compiler)
• the Server and Client Composition Patterns (not enforced by ESLint, so you really just need to know these)

from skeleton.

Hey @eagerestwolf, first of all, thanks for the write up! I am one of the people that helped authoring the React components (Accordion and Progress). So any tips are genuinly much appreciated since I have no prior experience with React (apart from watching the occational Theo video).

Also you stated:

Also also, I had to rollback the tsconfig paths for the react package. It broke the docs site. Apparently, Vite and vite-plugin-pagefind do not play nicely with tsconfig files.

I am the author of vite-plugin-pagefind and I am very interested in what you mean with "do not play nicely with tsconfig files" since the plugin never interacts with any typescript stuff as it's just a Vite plugin. I would love to fix this issue if it turns out to be an actual bug.

You also said:

Also, I removed the .js extensions from the imports. It's not needed when using TypeScript, and everything works fine without it.

I'm not sure I agree with this, I had learned that .js actually helps improve performance during development/build but maybe that's not relevant anymore, we should investigate this a bit further.

Like Chris said, I think opening a seperate issue and PR for each problem is best where we can deepdive into each problem in isolation.

from skeleton.

Thanks @eagerestwolf, two things of note.

First off, make sure you've read through this post. This outlines my availability, especially this week:

• #2729

Second, per this

Third, it's generally not a good idea to ship an entire React app in a component library.

This is a known issue and already on our radar to resolve. It was our first time packaging a Vite/React app for NPM and we mistakenly shipped more than just the components.

I'm interested in learning more about the benefits of the syntactical changes you mentioned, but performance should be priority number one. We've made things work, now we make them work better. That's where I assumed we had the biggest issue and what I'd like to tackle first. As you mention, it's likely due to re-rendering and/or general memoization issues.

My suggestion is we tackle each of the issues brought up above in individual dedicated PRs if possible. Handle it in "waves". That way we can keep changes small, surgical, and isolated.

No rush if you wish to help contribute directly, especially given I'll be out for the next week, but I look forward to tackling these issues soon.

from skeleton.