-
Notifications
You must be signed in to change notification settings - Fork 987
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
Migrate to nextjs 15 #1036
base: main
Are you sure you want to change the base?
Migrate to nextjs 15 #1036
Conversation
Thanks @rin-st I will take a look and test. Maybe some of the extensions have to be updated too, mainly if they use a page param, right? |
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.
The code looks good and it seems do not have any missing migration stuff.
Tested and it's working fine.
The first time the compilation took me about 16 seconds and next one about 8 seconds, so it doesn't be an issue there.
I'm getting a deprecation warning after the compilation is done:
✓ Compiled / in 8.1s (7831 modules)
(node:142505) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
Thanks!!
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.
Been testing it on Windows and LGTM! Nice job @rin-st!!🙌
Had a problem when tried to yarn build
but it worked well after deleting my .next
folder
packages/nextjs/app/debug/_components/contract/utilsDisplay.tsx
Outdated
Show resolved
Hide resolved
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.
Thanks @rin-st! Working great! did some testing and seems great to me! Just added a small comment https://github.com/scaffold-eth/scaffold-eth-2/pull/1036/files#r1926345215 but happy to merge it 🙌
Lol NextJs showing error overlay now for all types of error seems a bit strange, in previous version it was only showing it for "Unhandled error".
Especially the error of Grammarly/colorzilla extension:
Rauch response on it: https://x.com/rauchg/status/1871268516508381412
Similarly when chain is not started it show the error overlay, lol we can't do anything about since nextjs plans to do it that way.
I was researching about this error overlay seems like they are improving this error overlays a lot checkout their changelog the "[DevOverlay]" seem good, probably they will release that in 15.2.x version but looks nice.
Thanks for reviews !
Strange, I don't get that error
Oh yes, forgot to mention it. I don't like it too but dont know for now how to fix this
Probably we need to wait until 15.2 then because for now it shows red error button for every contract error, even when there is a |
lol I figured this out. It shows error button for every |
I think its shows up if you use latest node version?
Ohh I think it won't be solved in 15. 2 as well, "I think" because (I wasn't able to find the changeset were they discussed) NextJs plan to move this way in future showing console.error in that overlay as well. In 15.2 I think they are improving the styles of it a lot and making it more better.
Umm regarding this, probably I think we shouldn't change all console.error to console.warning, since its only show in dev environment its fine? Maybe instead of converting all console.error to console.warning we only change the sensible ones? Like for example Showing write contract error, we shouldn't give overlay (i.e console.error) because we are already showing toast and we could give full error in console with warn? maybe we can wait for 15.2 @rin-st and see what say? if it improves? Because I see they are moving fast with 15.2 canary and probably release new version soon? If the overlay is still their for console.error then we take the decision where to put console.war and where to use console.error? |
Yes, let's wait! Thanks! |
Ohh Nex 15.2 is out https://nextjs.org/blog/next-15-2 🙌 |
Updated to next 15.2 and reverted all the At first, it showed me Grammarly error, probably because of the cache. But now it shows only console.errors but it not overlays on every error and only shows it on the left bottom corner nicely Screen.Recording.2025-03-02.at.16.36.48.movalso, it can be configured on "N" icon click, for example to disable it for the app session I think it's good for us |
Thanks @rin-st!! Yeah that grammrly error is bad :( I get it evertime I spin up SE-2. Also the bad part is that it hides other errors below it, as well if you click "X" then you won't get notified about error :( Screen.Recording.2025-03-06.at.6.13.43.PM.mov^ I mean this will happen to all NextJs users so probably that might be get comfortable with it but not sure if SE-2 should do anything about it not sure if we can do something or not (probably adding Also I think we need to re-think about console.erroring now? Because take for example "Debug page" (this is util by SE-2 for developers and we don't want them to deep dive init) but if the write reverts it shows the next error in bottom checkout about video :( maybe we shouldn't do that? scaffold-eth-2/packages/nextjs/app/debug/_components/contract/WriteOnlyFunctionForm.tsx Lines 47 to 66 in c5c8139
Since we are already handling error (try / catch) probably we just "log" it instead of again erroring out forward? So alteast all the SE-2 utils like faucet, blockexplorer shouldn't do Just putting it out here and would love to hear others views / opinions 🙌 cc @carletex too |
Thanks Shiv!
Yes, I think it's ok to ignore it for now since it's a default nextjs behavior
What do you think about changing it to |
Next 15 what's new and migration guides here
codemods used:
react:
main migration codemod
types-react-codemod
nextjs:
async request api
Plus changed the next config file
Note: After updating deps, compile times for se-2 homepage were crazy 60-100 seconds, but then the problem eventually disappeared. Currently compile times in dev mode for me is like 8-13sec. Hopefully, for you it will work fine from the start
Things I thought about but decided it's not worth it
--turbo
are almost the same . I think it's better just to wait until it will become default for nextjsTODOs after migration to next15
Fixes #1033