-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: update vueschool banner script url for adblocker prevention #2550
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
feat: update vueschool banner script url for adblocker prevention #2550
Conversation
WalkthroughUpdated the external Vue School top banner script URL in the docs VitePress shared config. The script tag’s attributes (async, type) and the rest of the configuration remain unchanged. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 |
✅ Deploy Preview for vue-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/docs/.vitepress/config/shared.ts (1)
111-115
: Add media.bitterbrains.com to CSP/script-src (Netlify headers)repo has netlify.toml at the root but no Content-Security-Policy/script-src or allowlist for media.bitterbrains.com was found; the script in packages/docs/.vitepress/config/shared.ts (lines 111–115) loads https://media.bitterbrains.com and will be blocked if CSP is enforced. Add the host to your site CSP (netlify.toml or _headers). Example (netlify.toml):
[[headers]]
for = "/*"
[headers.values]
Content-Security-Policy = "default-src 'self'; script-src 'self' https://media.bitterbrains.com;"
🧹 Nitpick comments (3)
packages/docs/.vitepress/config/shared.ts (3)
112-114
: Avoid@ts-expect-error
: emit boolean attribute as empty string (consistent with Fathom).VitePress head attrs are typed as strings; using
async: ''
removes the need for the TS override and matches the style used fordefer
.Apply this diff:
- // @ts-expect-error: vitepress bug - async: true, + async: '',
111-115
: Optional: usedefer
for deterministic execution order.If the banner script doesn’t need to run before parsing finishes,
defer
provides more predictable timing thanasync
. Keepasync
if early paint of the banner is desired.
111-115
: Nit: preconnect/dns-prefetch to reduce first-load latency.Add resource hints for the new host.
Outside this hunk, append in
head
:['link', { rel: 'preconnect', href: 'https://media.bitterbrains.com' }], ['link', { rel: 'dns-prefetch', href: '//media.bitterbrains.com' }],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/docs/.vitepress/config/shared.ts
(1 hunks)
🔇 Additional comments (2)
packages/docs/.vitepress/config/shared.ts (2)
111-111
: LGTM — URL swap matches the PR goal.The change aligns with the adblocker-prevention objective and keeps the rest of the config intact.
111-111
: Confirm query param name change (affiliate
→from
).Previous URL used
affiliate=vuerouter
; new one usesfrom=vuerouter
. Please verify with the vendor thatfrom
is recognized for attribution/tracking.
This PR is to update Vue School Banner script to https://media.bitterbrains.com/main.js to prevent adblockers.
Summary by CodeRabbit