- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33
Fix errors on @HofmannZ/feature/nextjs-15-compatibility branch #181
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
base: @HofmannZ/feature/nextjs-15-compatibility
Are you sure you want to change the base?
Fix errors on @HofmannZ/feature/nextjs-15-compatibility branch #181
Conversation
0a73ff2    to
    e57b82c      
    Compare
  
    | Hi! Thanks for contributing. I noticed you’re not awaiting some promises, which could lead to race conditions. Could you take a look and fix that? | 
| Hi, @HofmannZ 
 I guess you meant my 2nd commit of this PR which removed async from function signature and removed await like below. -export const PublicEnvScript: FC<PublicEnvScriptProps> = async ({ nonce }) => {
-  await connection(); // Opt into dynamic rendering
+export const PublicEnvScript: FC<PublicEnvScriptProps> =  ({ nonce }) => {
+  connection(); // Opt into dynamic renderingbefore removing async from signature (just 1st commit ), I got lots of errors as cited in the end of this comment when I checked current development branch, I found  dev branch uses non async style function as below next-runtime-env/src/script/public-env-script.tsx Lines 29 to 35 in 536fc86 
 with 2nd commit, It passed without errors without 2nd commit, lots of errors:  | 
| @HofmannZ if you prefer, I can push without 2nd commit. but note that it still have CI errors.. | 
e57b82c    to
    2e7869d      
    Compare
  
    | @HofmannZ I finally succeeded to build next-runtime-env with next15+react19 and push re-based version of my branch. 1st commit fixes ReferenceError on jest,  NEW 2nd commit fixes other errors on build phase. 3rd commit fixes the above commented errors (async and act matters ) as first aid. | 
| 
 The reason I needs to remove asyn/await words from some codes is: I had tested the code of this PR and worked with my software as expected, even without async/await. anyway, what's the process needed for getting this PR merged? | 
| FWIW: I have been using this branch for a week or so in our test environment and haven't seen any issues. But i did need to bump the peerDependencies to 15/19 also. | 
| Any updates on this? nextjs 14 LTS ends in a couple of days | 
| As of yesterday NextJS 14 is out of LTS. Is there any plans to merge this or release it as a separate package? | 
| unfortunately, I dont have time to maintain this branch as separate package. If maintainer of this repo cannot merge nor release, I want somebody to maintain... | 
fix errors on @HofmannZ/feature/nextjs-15-compatibility branch