#287 Upgrade next to v15 #289
Conversation
nevendyulgerov
added
the
Type: Chore
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…s to latest version
nevendyulgerov
force-pushed
the
feature/287-upgrade-next-to-v15
branch
from
53525bd to
7458434
Compare
| export const useElementVisibility = <T extends Element>({ | ||
| element, | ||
| export const useElementVisibility = ( | ||
| element: HTMLDivElement | null, |
There was a problem hiding this comment.
That is an odd type narrowing saying that the only element type accepted would be div elements or null. But in theory, it was supposed to accept any valid HTML element or null. I checked the code and played around, and it would take a ref typed as HTMLInputElement without any squiggly red lines (the project is using typescript 5.3.2), which causes other problems in terms of modifying that hook as the intellisense will display incorrect attributes.
I believe the element being a generic type that extends the correct ParentType is necessary. The refactoring going down one level i.e. RefObject<T> to Element does not look like a problem.
There was a problem hiding this comment.
Hey @nevendyulgerov, I am not saying to put it back to what it was (like you're doing here with a bit of difference cbf28cb). As I mentioned refactoring to one level down by receiving the HtmlElement or null instead of the RefObject is fine, but my highlighting was about the type narrowing to Div, instead of a more generic one e.g. by using generics.
There was a problem hiding this comment.
Hi, @brunomenezes , I'm not sure I understand. The code does use generics as it did before with the changes done in cbf28cb.
Please let me know if you want further changes related to this.
There was a problem hiding this comment.
To clarify further, the comment was not about the refactoring from the object param to the function param, just the type narrowing.
I upgraded next to
v15and react tov19for thewebapp which resulted in library upgrades in:packages/ui- upgraded thereactandtesting-libraryversionsapps/workshop- upgraded thereactandstorybookversionsI also tested the app through the docker container and didn't find any issues.
Additionally, I disabled all metamask e2e tests as those continue to be flaky. I'll look into those issues in upcoming tasks.