Fix FlatGeobuf deserialization error and add cache-busting#512
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR patches Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/atlas/atlas-url.ts (1)
23-24: Consider handling undefinedPUBLIC_APP_VERSION.
PUBLIC_APP_VERSIONis typed as optional (string | undefined). If the Vite define injection fails or the build runs outside npm, the URL would contain?v=undefined, which still works but defeats the cache-busting purpose since it won't change across deploys.🛡️ Optional defensive fallback
const appVersion = import.meta.env.PUBLIC_APP_VERSION - const availableFieldsUrl = `${datasetsUrl}/fields/nl/${year}/${version}.fgb?v=${appVersion}` + const availableFieldsUrl = `${datasetsUrl}/fields/nl/${year}/${version}.fgb?v=${appVersion ?? Date.now()}`Alternatively, you could throw an error at build time if the version is missing, or use a fallback like a build timestamp.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/blocks/atlas/atlas-url.ts` around lines 23 - 24, The code uses import.meta.env.PUBLIC_APP_VERSION directly (assigned to appVersion and interpolated into availableFieldsUrl) without handling the optional undefined case; update the appVersion assignment to check import.meta.env.PUBLIC_APP_VERSION and if undefined either (a) substitute a deterministic fallback (e.g., a build timestamp or process.env commit hash) or (b) fail early at build time, and then use that safe value when creating availableFieldsUrl; ensure the change touches the appVersion binding and the availableFieldsUrl template so the query param never becomes "?v=undefined".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@fdm-app/app/components/blocks/atlas/atlas-url.ts`:
- Around line 23-24: The code uses import.meta.env.PUBLIC_APP_VERSION directly
(assigned to appVersion and interpolated into availableFieldsUrl) without
handling the optional undefined case; update the appVersion assignment to check
import.meta.env.PUBLIC_APP_VERSION and if undefined either (a) substitute a
deterministic fallback (e.g., a build timestamp or process.env commit hash) or
(b) fail early at build time, and then use that safe value when creating
availableFieldsUrl; ensure the change touches the appVersion binding and the
availableFieldsUrl template so the query param never becomes "?v=undefined".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2e578470-631a-4d79-b094-bef9dc452b3c
📒 Files selected for processing (3)
.changeset/tiny-friends-doubt.mdfdm-app/app/components/blocks/atlas/atlas-sources.tsxfdm-app/app/components/blocks/atlas/atlas-url.ts
| // browser cache key on each app deploy, clearing any corrupted HTTP Range | ||
| // response entries that may have accumulated in the browser cache. | ||
| const appVersion = import.meta.env.PUBLIC_APP_VERSION | ||
| const availableFieldsUrl = `${datasetsUrl}/fields/nl/${year}/${version}.fgb?v=${appVersion}` |
There was a problem hiding this comment.
I would namespace this search param like, for example, ?fdm-app-v=PUBLIC_APP_VERSION. This minimizes the chances of collision with valid Google Cloud search parameters.
You might not want to make it obvious that this is fdm-app making the request, but I don't know why this might be a concern.
3 participants
Summary by CodeRabbit
Bug Fixes
Closes #509