refactor(tools): svg-converter cleanup, shared tool skeletons, ytm cookie script - #85
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideRefactors the SVG converter to distinguish warnings from auto-fix notices, auto-derive viewBox/size behavior, and surface these in UI; centralizes tool loading skeletons into a shared module and wires them into the tool renderer and individual tools; and adds an interactive script to safely update the YTM cookie in both .env and Vercel production. Flow diagram for the YTM cookie update scriptflowchart TD
Developer[[Developer]] --> Script[update-ytm-cookie.sh]
Script -->|prompt cookie header| Developer
Developer -->|paste YTM_COOKIE| Script
Script --> PythonHelper[Inline python3 helper]
PythonHelper --> EnvFile[.env file]
EnvFile -->|YTM_COOKIE updated| Script
Script --> VercelEnv[vercel env commands]
VercelEnv -->|remove existing YTM_COOKIE| VercelEnv
VercelEnv -->|add new YTM_COOKIE| VercelProduction[Vercel production env]
Script -->|prompt redeploy| Developer
Developer -->|confirm redeploy| Redeploy[vercel redeploy / vercel --prod]
Redeploy --> VercelProduction
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe SVG converter now distinguishes notices from warnings and reports auto-fixes. Miscellaneous tools use specific SSR loading skeletons. A YTM cookie synchronization script was added. Package-page in-page navigation was removed. ChangesSVG converter notices
Tool-specific loading skeletons
YTM cookie synchronization
Package page navigation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
… skeletons, ytm cookie script
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Switching all tools in
tool-renderertossr: truechanges the rendering behavior for previously client-only tools likecoordinate-markerandhemelsbreed; double-check that these components (and their dependencies like Leaflet/geolocation) are safe to render on the server and won’t hitwindow/browser-only APIs during SSR. - The new
scripts/update-ytm-cookie.shassumes an existing.envfile and will fail if it’s missing; consider creating the file if it doesn’t exist (or handling that error) before callingenv.read_text()in the embedded Python.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Switching all tools in `tool-renderer` to `ssr: true` changes the rendering behavior for previously client-only tools like `coordinate-marker` and `hemelsbreed`; double-check that these components (and their dependencies like Leaflet/geolocation) are safe to render on the server and won’t hit `window`/browser-only APIs during SSR.
- The new `scripts/update-ytm-cookie.sh` assumes an existing `.env` file and will fail if it’s missing; consider creating the file if it doesn’t exist (or handling that error) before calling `env.read_text()` in the embedded Python.
## Individual Comments
### Comment 1
<location path="scripts/update-ytm-cookie.sh" line_range="14-23" />
<code_context>
+ exit 1
+fi
+
+python3 - "$COOKIE" <<'PY'
+import sys, re, pathlib
+cookie = sys.argv[1].strip()
+env = pathlib.Path('.env')
+text = env.read_text()
+pattern = re.compile(r'^YTM_COOKIE=.*$', re.M)
+if pattern.search(text):
+ text = pattern.sub(lambda _: f'YTM_COOKIE={cookie}', text)
+else:
+ text = text.rstrip('\n') + f'\nYTM_COOKIE={cookie}\n'
+env.write_text(text)
+print('Updated .env')
+PY
</code_context>
<issue_to_address>
**suggestion:** The script assumes `.env` exists and that `vercel env` flags behave as expected; consider hardening this flow.
Two fragilities to address: (1) `pathlib.Path('.env').read_text()` will raise if `.env` is missing, aborting the script without a clear explanation. Add an existence check and either create the file or emit a clear error. (2) The `vercel env add` call’s use of `--value` and `--yes` may not be supported across all Vercel CLI versions. Confirm the expected CLI behavior and, if needed, use a documented non-interactive pattern or fall back to interactive mode.
Suggested implementation:
```
python3 - "$COOKIE" <<'PY'
import sys, re, pathlib
cookie = sys.argv[1].strip()
env = pathlib.Path('.env')
try:
text = env.read_text()
except FileNotFoundError:
text = ''
except OSError as exc:
print(f"Failed to read .env: {exc}", file=sys.stderr)
sys.exit(1)
pattern = re.compile(r'^YTM_COOKIE=.*$', re.M)
if pattern.search(text):
text = pattern.sub(lambda _: f'YTM_COOKIE={cookie}', text)
else:
text = text.rstrip('\n') + f'\nYTM_COOKIE={cookie}\n'
try:
env.write_text(text)
except OSError as exc:
print(f"Failed to write .env: {exc}", file=sys.stderr)
sys.exit(1)
print('Updated .env')
PY
```
```
vercel env rm YTM_COOKIE production --yes 2>/dev/null || true
if ! printf '%s\n' "$COOKIE" | vercel env add YTM_COOKIE production --yes; then
echo 'Failed to push YTM_COOKIE to Vercel. You may need to run `vercel env add YTM_COOKIE production` manually.' >&2
else
echo 'Pushed YTM_COOKIE to Vercel production.'
fi
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+14
to
+23
| python3 - "$COOKIE" <<'PY' | ||
| import sys, re, pathlib | ||
| cookie = sys.argv[1].strip() | ||
| env = pathlib.Path('.env') | ||
| text = env.read_text() | ||
| pattern = re.compile(r'^YTM_COOKIE=.*$', re.M) | ||
| if pattern.search(text): | ||
| text = pattern.sub(lambda _: f'YTM_COOKIE={cookie}', text) | ||
| else: | ||
| text = text.rstrip('\n') + f'\nYTM_COOKIE={cookie}\n' |
Contributor
There was a problem hiding this comment.
suggestion: The script assumes .env exists and that vercel env flags behave as expected; consider hardening this flow.
Two fragilities to address: (1) pathlib.Path('.env').read_text() will raise if .env is missing, aborting the script without a clear explanation. Add an existence check and either create the file or emit a clear error. (2) The vercel env add call’s use of --value and --yes may not be supported across all Vercel CLI versions. Confirm the expected CLI behavior and, if needed, use a documented non-interactive pattern or fall back to interactive mode.
Suggested implementation:
python3 - "$COOKIE" <<'PY'
import sys, re, pathlib
cookie = sys.argv[1].strip()
env = pathlib.Path('.env')
try:
text = env.read_text()
except FileNotFoundError:
text = ''
except OSError as exc:
print(f"Failed to read .env: {exc}", file=sys.stderr)
sys.exit(1)
pattern = re.compile(r'^YTM_COOKIE=.*$', re.M)
if pattern.search(text):
text = pattern.sub(lambda _: f'YTM_COOKIE={cookie}', text)
else:
text = text.rstrip('\n') + f'\nYTM_COOKIE={cookie}\n'
try:
env.write_text(text)
except OSError as exc:
print(f"Failed to write .env: {exc}", file=sys.stderr)
sys.exit(1)
print('Updated .env')
PY
vercel env rm YTM_COOKIE production --yes 2>/dev/null || true
if ! printf '%s\n' "$COOKIE" | vercel env add YTM_COOKIE production --yes; then
echo 'Failed to push YTM_COOKIE to Vercel. You may need to run `vercel env add YTM_COOKIE production` manually.' >&2
else
echo 'Pushed YTM_COOKIE to Vercel production.'
fi
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tool-skeletons.tsxand simplified per-tool loading states (coordinate-marker, diff-checker, find-replace, hemelsbreed, tool-renderer)scripts/update-ytm-cookie.shto rotateYTM_COOKIEin.envand Vercel production (prompts interactively, no secrets committed)Testing
vitestsvg-converter suite: 15/15 passtsc --noEmit: cleanSummary by Sourcery
Improve SVG converter auto-fix behavior and UI feedback, standardize loading skeletons across miscellaneous tools, and add a script to manage the YTM_COOKIE in local and Vercel environments.
New Features:
Enhancements:
Deployment:
Tests:
Summary by CodeRabbit
New Features
viewBoxvalues when possible and preserves applied fixes.Bug Fixes
Style