-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
more guides #6357
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
base: main
Are you sure you want to change the base?
more guides #6357
Conversation
📝 WalkthroughWalkthroughTwo new comprehensive guides are added to the React documentation covering security headers implementation in middleware and multi-tenant subdomain routing patterns. The configuration file is updated to reference these new guides. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
View your CI Pipeline Execution ↗ for commit c127dce
☁️ Nx Cloud last updated this comment at |
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: 1
🤖 Fix all issues with AI agents
In @docs/start/framework/react/guide/multi-tenant-routing.md:
- Around line 601-604: The fenced code block that shows the /etc/hosts entries
(the block containing "127.0.0.1 acme.localhost" and "127.0.0.1
globex.localhost") is missing a language tag; update that triple-backtick fence
to include a language identifier (e.g., add "text" so it becomes ```text) to
satisfy markdownlint and improve rendering.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/start/config.jsondocs/start/framework/react/guide/multi-tenant-routing.mddocs/start/framework/react/guide/security-headers.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use relative links to
docs/folder format (e.g.,./guide/data-loading) for internal documentation references
Files:
docs/start/framework/react/guide/security-headers.mddocs/start/framework/react/guide/multi-tenant-routing.md
🪛 markdownlint-cli2 (0.18.1)
docs/start/framework/react/guide/multi-tenant-routing.md
601-601: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Preview
🔇 Additional comments (3)
docs/start/config.json (1)
152-159: LGTM! Well-organized guide additions.The new Security Headers and Multi-Tenant Routing guide entries are appropriately placed after the Authentication entry, logically grouping security and routing concerns. The paths follow the established pattern and align with the new documentation files.
docs/start/framework/react/guide/security-headers.md (1)
1-462: Excellent comprehensive security guide!This guide provides thorough coverage of security headers implementation in TanStack Start. The content is well-structured, covering:
- Global and per-route middleware approaches
- CSP with nonce support for inline scripts
- Development vs production configurations
- CORS for API routes
- Production-ready examples
The internal documentation links (lines 460-462) correctly use the relative format as specified in the coding guidelines.
docs/start/framework/react/guide/multi-tenant-routing.md (1)
1-615: Excellent comprehensive multi-tenant routing guide!This guide provides thorough coverage of multi-tenant subdomain routing in TanStack Start. The content is well-structured, covering:
- Subdomain extraction for both localhost and production
- Database lookups with caching strategies (in-memory and Redis)
- Type-safe context propagation
- Handling unknown tenants with multiple approaches
- Complete working examples
The internal documentation links (lines 613-615) correctly use the relative format as specified in the coding guidelines.
| ``` | ||
| 127.0.0.1 acme.localhost | ||
| 127.0.0.1 globex.localhost | ||
| ``` |
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.
Add language specification to code fence.
The code block showing /etc/hosts configuration is missing a language identifier, which is flagged by markdownlint.
📝 Proposed fix
-```
+```text
127.0.0.1 acme.localhost
127.0.0.1 globex.localhost
</details>
Based on static analysis hints (markdownlint).
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
601-601: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @docs/start/framework/react/guide/multi-tenant-routing.md around lines 601 -
604, The fenced code block that shows the /etc/hosts entries (the block
containing "127.0.0.1 acme.localhost" and "127.0.0.1 globex.localhost") is
missing a language tag; update that triple-backtick fence to include a language
identifier (e.g., add "text" so it becomes ```text) to satisfy markdownlint and
improve rendering.
|
|
||
| ## Handling Unknown Tenants | ||
|
|
||
| When a subdomain does not match a known tenant, you have several options: |
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.
| When a subdomain does not match a known tenant, you have several options: | |
| When a subdomain does not match a known tenant, you have few options: |
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.
Mostly so we aren't talking having the too many options syndrome.
| export const Route = createFileRoute('/_tenant')({ | ||
| beforeLoad: async ({ context }) => { | ||
| if (context.tenantNotFound) { | ||
| throw redirect({ to: '/workspace-not-found' }) |
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.
| throw redirect({ to: '/workspace-not-found' }) | |
| // redirect to a page with a CTA to sign up for your app | |
| throw redirect({ to: '/workspace-not-found' }) |
| }) | ||
| ``` | ||
|
|
||
| ```tsx |
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 we need to show them what a possible error page with a CTA would look like. Unless we're expecting them to copy-paste this, but given the kind of guide this is, I wonder if this is what that kind of user is looking for.
|
|
||
| ## Content-Security-Policy (CSP) | ||
|
|
||
| CSP prevents cross-site scripting (XSS) attacks by controlling which resources can be loaded. |
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.
Maybe move this to the top of the page as a GFM callout (maybe a NOTE or INFO), instead of having its own heading for just this little one-liner bit of content that doesn't have any sub-headings under it.
| Security headers protect your application against common web vulnerabilities. TanStack Start provides several approaches for setting headers: | ||
|
|
||
| - **Global request middleware** applies headers to all responses (SSR, server functions, server routes) | ||
| - **Server route handlers** set headers for specific API endpoints |
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.
The headings not matching the these list items did confuse me for a second there when doing a read through.
Perhaps keep this guide a bit a more concise?
- Here are ways you can apply CSP (using an imported set of headers so the code blocks are kept short) in TanStack Start - like using global middleware, server routes, per-route, etc.
- Here are some common recipes or here's a good starter you can build upon as your app needs it.
- Here are some tools for testing.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.