refactor(http/unstable): improve Cache Control parsing and types#7087
refactor(http/unstable): improve Cache Control parsing and types#7087bartlomieju merged 9 commits intodenoland:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7087 +/- ##
=======================================
Coverage 94.42% 94.43%
=======================================
Files 630 630
Lines 50533 50579 +46
Branches 8964 8977 +13
=======================================
+ Hits 47717 47763 +46
Misses 2247 2247
Partials 569 569 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Cache-Control helpers
bartlomieju
left a comment
There was a problem hiding this comment.
Nice bug fixes and test coverage. The backslash escape fix, empty field-name normalization, and staleIfError addition are all correct. Main concerns are around the type restructuring and Set removal — see inline comments.
|
Agreed. Here's what I did:
|
bartlomieju
left a comment
There was a problem hiding this comment.
The bug fixes here look great — backslash escape handling, empty field-name normalization, and staleIfError on RequestCacheControl are all correct.
However, it looks like the changes you mentioned in your comment (restoring CacheControlBase, Set-based dedup, and the RFC comment in default) haven't been pushed yet — the diff still shows the old state. Could you push those?
|
Oh my.. 🫣 |
There was a problem hiding this comment.
Previous round's feedback was addressed. The remaining changes are solid:
staleIfErrortoCacheControlBase: Correct per RFC 5861 §3-4 (valid in both request and response).CacheControlmapped type: Cleaner than the union — no more type assertions needed to access non-shared fields. The internalAllCacheControlFieldsalias is correctly removed since the public type now serves the same purpose.- Backslash escape handling in
splitDirectives: Correctly skips any character after\while inside quotes. Edge cases (trailing backslash,\\"sequences) all handled correctly. no-cache=""/private=""normalization: Correct — an empty field-name list is semantically equivalent to bareno-cache/private.
Uh oh!
There was an error while loading. Please reload this page.