-
Notifications
You must be signed in to change notification settings - Fork 511
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(netlify, vercel): explicit server rendering with disabled cache/swr #829
Conversation
Codecov Report
@@ Coverage Diff @@
## main #829 +/- ##
==========================================
+ Coverage 69.83% 69.93% +0.10%
==========================================
Files 57 57
Lines 5291 5312 +21
Branches 586 594 +8
==========================================
+ Hits 3695 3715 +20
Misses 1588 1588
- Partials 8 9 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
([_, value]) => | ||
value.cache === false || | ||
(value.cache && value.cache.swr === false) || | ||
(value.cache && (value.cache?.static || value.cache?.swr)) |
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.
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.
I don't think that simplification is right:
- it drops a check for
static
- we do care whether cache is explicitly
false
, in which case we want to write the rule, but if it's undefined we shouldn't force server rendering, 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.
Yeah. The main problem is that we are not deeply normalizing the route rules at the moment and with undifined conditions, such simplification is wrong. Cache should be either false
or an object. And I'm thinking the static flag could be moved outside. Lets improve 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.
LGTM for vercel hotfix. But we really need to improve cache<>builder mapping system and make it generic and better testable in the future...
π Linked issue
β Type of change
π Description
At the moment if you have a default cache rule, you can't override it negatively at a more specific level.
This PR should resolve that.
π Checklist