-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#287 Upgrade next to v15 #289
Merged
Merged
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
70d9caa
feat(apps/web): Upgrade next to v15 and react to v19
nevendyulgerov 6870c67
feat(apps/workshop): Upgrade react to v19 and storybook packages to v…
nevendyulgerov 000768b
feat(apps/workshop): Upgrade react to v19 and testing-library package…
nevendyulgerov 7a14826
feat(apps/web): Remove deprecated config option
nevendyulgerov 83f3b8b
feat(apps/web): Make params retrieval async
nevendyulgerov 5e69640
chore(apps/web): Use act from 'react' package instead
nevendyulgerov 0310d23
chore: Update lock file
nevendyulgerov 6cffbc5
chore(apps/web): Dynamically import the Shell component in a client c…
nevendyulgerov 7458434
chore(apps/web): Reformat files
nevendyulgerov 11f74c0
chore(apps/web): Simplify useElementVisibility hook
nevendyulgerov 9b2c63b
test(apps/web): Skip more metamask tests
nevendyulgerov 39e3731
test(apps/web): Skip more metamask tests
nevendyulgerov cbf28cb
chore(apps/web): Refactoring
nevendyulgerov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
"use client"; | ||
import dynamic from "next/dynamic"; | ||
|
||
export const Shell = dynamic(() => import("./shell"), { | ||
ssr: false, | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is an odd type narrowing saying that the only element type accepted would be
div
elements ornull
. 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 asHTMLInputElement
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 theRefObject
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify further, the comment was not about the refactoring from the object param to the function param, just the type narrowing.