-
Notifications
You must be signed in to change notification settings - Fork 917
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
Update CLI with latest main #555
Conversation
Co-authored-by: CJ <53488449+0xChijoke@users.noreply.github.com>
* update typescript and viem version * update next config
* feat: add indexed args to events * fix: listener arg type * fix log.args for useScaffoldEvenSubsriber hook --------- Co-authored-by: Shiv Bhonde | shivbhonde.eth <shivbhonde04@gmail.com>
Curious about why this happened, I did some digging, and here's my finding : SE-2 When you create a new template from this PR, its notice that it create two blocks one with version I also tried removing "~" from is that some other package is dependent on latest version of is this related somewhere to @rin-st comments => #541 (comment) (might be unrealted) For now, the solution seems #555 (comment) but just curious why this happened and this affects us in future |
Thanks for the investigation @technophile-04 ! Some more stuff that I found ( In CLI we have 2 diff versions because:
These two use a newer version of types/react (18.2.24). What I did then is to remove the So, it the vercel dep is gone, but not the clipboard one. I think this is exactly what @rin-st was talking about here: #541 (comment) Even if we install the same top dep (5.0.4) it uses a newer version of
😢 So not having a lockfile is tricky. We can remove A decent solution:
Not ideal, but better than nothing. Thoughts? -- Also check this comment: #555 (comment) This could help for errors related to |
Tysm @carletex !! Just really awesome explanation love it!!, almost all my doubts are cleared nicely and now it makes complete sense too!! 🙌
Yes, I agree 💯 Just very last one doubt : Even If we remove For the above reason, we are planning to set tests but won't there be a case lets suppose : We are using library Now lets suppose there is another library we are using This will affect It will be really hard for us to debug too right ? Since If we use Just trying to make the point that debugging will get a lot harder if the wrong internal dependencies are installed, but really don't know If I am making sense here (and might be wrong) and this might not even happen Really sorry for overthinking this too much please feel free to ignore the above since its kinda made up the situation in my head 😅 but yeah even I feel removing |
@technophile-04 I think I understand what you are saying. doing
If we have good automated tests, I guess we could detect that something is not working (deploy, start, checking types, etc), right? And the we can investigate the root of it and fix it (it'll prolly be a versions that needs to be updated) I think I'm gonna update @types to ^ for now.
Maybe we can merge this one? I'll send the update in a bit. |
This is working for me @technophile-04 If it works for you.. let's merge and publish! |
Great investigations! Experimented with it too but didn't find something better
I thought: child dep breaks our dep -> our dep breaks our app. But it's even worse. child dep breaks our app 🤷♂️ . I didn't know about that problem before and thought package managers can manage it and use different versions needed for our dep and child dep. But it's not always true. Also I thought
It works for me, but I believe we also need to think later about removing that
Not sure how to do it, but if it's possible why not. @technophile-04 sorry I don't know the answer to your question. Will try to read it again tomorrow 😄 Thanks! ps. FYI, if you didn't see it yet facebook/react#24304 (comment) |
Yup yup 💯💯 I rarely see lib using Tysm for answering 🙌
No worries Rinat 😆 Just ignore it trust me it was very highly kind of unreal situation cooked up in my head and lol just dumped it in that comment, but yeah as Carlos mentioned libraries mentioning I tested it and works nicely also tried doign Just pushed a small tweak at foundry at 4f6fcb5 which we did at #531 and added chandeset 🙌 Tysm guys merging this! |
If you meant by In our case it creates two versions. The question for me how and why types from
Not sure. I think mostly same like other error. See stack trace -> find what's wrong |
Thank y'all for this discussion. Learning a lot! I think we should have this discussion open (maybe a new issue?) because this is something that we are going to face. I guess the base issue is not having a Something that we could explore in some cases => yarn resolutions (so we can set the version of any sub-dependency):
|
Yup 💯💯, Tysm Rinat 🙌 After reading alot of issues/discussions :
Yeah "*" considers newest version but only in case of If we would have used lol Also this @types/react error happes only when there are multiple versions of Looking at solutions, people seem rely on yarn resolution for this and we can do something like this : "resolutions": {
"@types/react": "18.2.25"
} I think we are good using
I think there were will long way before major version of But maybe by then we give people option to select different package manager like pnpm or bun 🙌 |
It would be good, with sum up of this pr discussion, or just with link to it
Agree
😄 yes, but we can forget about this issue when it will happen. And I'm afraid it can happen not only for react, but for smaller packages too. Hope I'm wrong
Thanks! Chatgpt deceived me 😄 . It said that it works for every package manager.
Tried to test it, pnpm resolves * and takes 18.0.21 so it works 🙌. |
main
Back-merge from 02dd022 to b33a87d. Check commits.I created a
cli-backmerge
branch fromcli
, where I mergedmain
and fixed conflicts. That way we can create this PR and check & test the PR before merging to CLI. Maybe we could add this to DEV_GUIDE md fileNote: Until main === cli, let's do this (back-merge main) once a week. If not, it gets messy.