FYI React 18 testing
Closed this issue · 7 comments
I wrote about using React 17 in a previous issue on my work github, after some initial testing with React 18, htwoo appears to work on it too, with some warnings that it's not supported same reason I submitted the issue about React 17. I think it is because of the atom design system that things seem to work in React 18.
I will try out every single atom, molecule, organism and let you know what works and doesn't work. Should know the results tomorrow/Sunday.
I am using React 18's strict mode. I will collect info on what works/doesn't using strict mode and let you know.
I will submit the example, when I have it complete.
The contributing guidelines state I should submit an issue before contributing, to get feedback.
Completed initial testing with the atoms, molecules, organisms mentioned in htwoo-react docs
Good news
Everything works even in strict mode.
Not so good news:
- The type definitions are wrong.
- HOOPersona
- HOODocumentCard
- HOODialog
- HOODialogContent
All need the type definition of children added eg
export interface ISampleInterface {
children?: React.ReactNode;
...OtherProps
}- HOOOptionList doesn't set the key attribute that React requires to work as expected. This will cause bugs for people.
- Setting up the CSS for a non-SPFx project is not easy to figure out. Docs around this would be required.
- Docs don't mention anything about accessibility, even though every component accepts passing in attributes, which is essential for accessibility, a quick note about that would go a long way to help adaption.
Conclusion
There is no need to test the HTML/CSS combinations for React 18 as they will just work even without the testing.
As it stands, this project is OK to say that it supports React 18.2, even with the mentioned issues. This is a better result than the original Fabric/Fluent/Northstar projects.
Waiting for feedback about this before submitting a PR for a non-SPFx React sample.
So first, Thank you so much for all your work on this... I will be VERY happy to update our React dependency to support up to 18.2... that is exciting news.
For item 1, issues with the typings... so, are you saying that children works for all the other items just not those four or are you just giving me an example? I have to say I'm not sure I follow. I tested HOOPersona and I do see proper typings for children when I add it as an attribute... maybe you mean something else.
For item 2, ahh, yep, that's a bug I had the key property on the wrong item, we'll get that update out in a new release.
For item 3, CSS for non-SPFx project yeah, I am in agreement, and this is on my list of things to talk to @StfBauer about as we're working on a project where this will be an issue, so we need to address it. It sounds like you have a solution of your own, can you elaborate?
For item 4... Accessibility... Yes understood, I have done some work to make sure some accessibility is managed by mimicking settings Stefan has done in the core library but as you point out you have full access to the root element attributes so pointing this out is helpful.
Hey thanks for the response, same person here lol.
I'm pretty sure for any version of React below 18, you can not put children into your type definitions and it's considered valid. In other words children is implicitly always in the typings. In React 18, children has to be explicitly set.
The files that I listed, are the ones that appear to have the wrong typings.
Yes I have a solution to loading the CSS for a non-SPFx project. It's pretty much the same as the spfx-html sample. Just have to make sure the CSS variables are set and everything works.
The files that I listed, are the ones that appear to have the wrong typings.
Since they have no explicit typings for children I am not sure I follow this statement. I can certainly add children to the inherited base that all the components use, it feels like that would fix the issues with React 18, but I really don't understand the "wrong" statement so I'm not sure this would fix the issue you're seeing.
By have a solution I meant something a bit more robust but yes, you have to define the theme slots. It's my intent to work with Stefan to build a default baseline theme definition (hopefully that's pared down) for non-SPFx implementations. We currently do include default Teams themes for light/dark/high contrast so I suspect the right answer is to give a fourth default that would be generic that could then be overridden and passed in the SPFxThemes class which maybe now should get a new name but for backwards compatibility I'm probably not going to do... then we'll have a way to provide a better sample. Need to look at that a bit deeper.
The wrong statement was pretty harsh words, I just wanted to get the feedback to you as fast as possible. Putting in the children definition explicitly will fix the type definitions for React 18.
React < 18 assumed every Component/Pure Component/Functional Component would have children and this meant they all by default had the children definition. React 18 makes no assumption, so all definitions must explicitly have the children property if they accept children.
I actually think taking the Theme Palette JSON that we have to provide to Fluent should be the approach to initializing the a non-SPFx project. But that topic should go into it's own thread/issue/project in Github.
If you have completed explicitly setting the children type definition on the inherited base then this issue can be closed as that solves all the problems.
You will need to change the peer dependencies again to include React 18, current released version of React 18 is 18.2.0.
I actually think taking the Theme Palette JSON that we have to provide to Fluent should be the approach to initializing the a non-SPFx project. But that topic should go into it's own thread/issue/project in Github.
Yep, this is our intent and I'm going to see if I can tackle that for the next release... stay tuned there.
Re React/Children... ok, cool... I just wasn't sure if I was missing some point you were trying to make. I've explicitly added children to all components through an inherited base properties so that should take care of that and I've updated the peer dependencies so I think we're good for the next release to support up through 18.2. Thanks again for your help with this, as you say I'm going to mark this as complete and close it with the next release.
Released, thanks again for all your hard work on this!