Skip to content
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

fix: nextjs incrementalCache usage fixed #79

Merged

Conversation

indatawetrust
Copy link

hi, with nextjs 10.2 version, the method of applying the incretalCache method has changed. current usage fixed

vercel/next.js#22384 (comment)

@indatawetrust
Copy link
Author

hi @thomvaill , do you have any comment for this pr?

@zenwork
Copy link

zenwork commented Jun 30, 2022

Is there a workaround to use this fix from this branch until the PR is merged and a new version is published? I am able clone and build but then I do not know how I should install my local build to use it

@indatawetrust
Copy link
Author

@zenwork you can use patch-package

@florian-hehlen
Copy link

If you are going to use patch-package please note that I got it working by using log4brains as a dev dependency rather than as a global dep. it also means that i had to wrap calls to log4brains into package.json scripts. But the good news is everything is working... even the github action to deploy to pages.

@mathiasmaerker
Copy link

@florian-hehlen How did you get it to work? I installed it localy as well and started via npm scripts but I get the same error. patching via patch-package didn't work either for me..
Any other suggestions?

@zenwork
Copy link

zenwork commented Jul 6, 2022

@mathiasmaerker , I did the following:

  • installed log4brains and patch-package as a package.json dependencies
  • run npm install
  • manually patch the one file in @log4brains/web based on this PR
  • followed the patch-package instructions to generate the patch file
  • added these scripts to the package.json
    "scripts": {
     "start": "log4brains preview",
     "build": "log4brains build",
     "new": "log4brains adr new",
     "postinstall": "patch-package"
    },
  • commit everything

from here on in I need to use npm start and npm run new instead of global commands

also make sure to uninstall your global installation

@mathiasmaerker
Copy link

@zenwork thanks for your help but unfortunately I tried that with no success

Just to be clear:

  • installed log4brains and patch-package as a package.json dependencies --> done
  • run npm install --> done
  • manually patch the one file in @log4brains/web based on this PR --> meaning edit in editor (node_modules@log4brains\web\dist\cli\commands\preview.js) and saving? then done

then I execute npx patch-package log4brains tried npx patch-package log4brains/@log4brains

but the result always is the same: ⁉️ Not creating patch file for package 'log4brains/@log4brains' ⁉️ There don't appear to be any changes.

so what am I doing wrong?

Thanks in advance

@zenwork
Copy link

zenwork commented Jul 6, 2022

it should be npx patch-package @log4brains/web

try that

@mathiasmaerker
Copy link

@zenwork I tried that to, but because @log4brains is not part of the package.json you get the error node_modules/@log4brains/web/package.json

@zenwork
Copy link

zenwork commented Jul 7, 2022

I don't know why you would have this issue. I did not have any trouble

@mathiasmaerker
Copy link

@zenwork Well, no clue either :( but thanks a lot for your help and effort 🥇

@Sobi-WanKenobi
Copy link

@thomvaill , hopefully you are feeling better and work is lightening up a bit. Any chance you will you be able to merge this PR soon? Thanks for the work here -- really like this project a lot.

@thomvaill
Copy link
Owner

Hey thank you so much @indatawetrust, you rock!
This makes me think that we definitely need to implement #77 ; that's why I updated your branch before merging by pinning the version of Nextjs to avoid this kind of issue in the future. But we definitely need to do this for all the depts. I will try to work on this next week.
Also, this issue was not caught by the scheduled non-regression tests because we only test the build and not the preview. Contributions are welcome for this, because I don't see an easy way to test this ;)

Again sorry for the delay... It's a bit difficult for me to find some time to work on the project, but hopefully your comments and contributions are really motivating, thank you for this!

@thomvaill thomvaill merged commit 4c9f2e1 into thomvaill:master Jul 8, 2022
@thomvaill
Copy link
Owner

Apparently there is an issue when upgrading from 1.0.0-beta.12 to 1.0.0-beta.13: the changes are not taken into account.
Do you confirm?
Let's discuss this on #80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants