feat: add copy cURL request option to playground copy button - #3141
feat: add copy cURL request option to playground copy button#3141JivusAyrus wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe playground now supports copying the active query or a generated cURL request. It adds header validation, environment substitution, feature-flag propagation, warning reporting, toolbar integration, and automated cURL tests. ChangesPlayground copy flow
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
studio/src/components/playground/copy-operation.tsx (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an explicit component return type.
CopyOperationdoes not declare a return type. Add the applicable React JSX element return type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio/src/components/playground/copy-operation.tsx` at line 16, Update the CopyOperation component declaration to explicitly return the applicable React JSX element type, while preserving its existing implementation and behavior.Source: Coding guidelines
studio/src/__tests__/playground-curl.test.ts (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse one template literal for the expected command.
These lines concatenate template literals. Use one template literal for the complete expected command.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio/src/__tests__/playground-curl.test.ts` around lines 17 - 19, Update the expected command construction in the playground curl test to use a single template literal for the complete curl command instead of concatenating multiple template literals. Preserve the existing URL, headers, line breaks, and serialized query payload.Source: Coding guidelines
studio/src/lib/playground-headers.ts (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd explicit return types to exported helpers.
validateHeadersandsubstituteHeadersFromEnvdo not declare return types. Add: voidand: Record<string, string>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio/src/lib/playground-headers.ts` around lines 1 - 9, Add explicit return types to the exported helpers validateHeaders and substituteHeadersFromEnv: declare validateHeaders as returning void and substituteHeadersFromEnv as returning Record<string, string>, preserving their existing implementations.Source: Coding guidelines
studio/src/lib/playground-curl.ts (1)
26-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anyin parsed JSON values.
parseJsonObjectreturnsRecord<string, any>, which propagates unchecked values into variables, headers, and the request body. ReturnRecord<string, unknown>and narrow values where string conversion or body construction requires it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio/src/lib/playground-curl.ts` around lines 26 - 81, Update parseJsonObject and related parsed JSON declarations to use Record<string, unknown> instead of any. Narrow unknown header values before string conversion, and ensure variables and request-body construction handle unknown values without unchecked typing while preserving the existing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@studio/src/components/playground/copy-operation.tsx`:
- Around line 89-91: Add an accessible name to the icon-only Button containing
CopyIcon by setting an appropriate aria-label such as “Copy operation,” while
preserving its existing variant, size, and styling.
In `@studio/src/lib/playground-curl.ts`:
- Around line 64-77: Update the header-building flow in the cURL generation
logic to validate each header name after substituteHeadersFromEnv and merging
extraHeaders, matching graphiQLFetch validation. Exclude invalid header names
such as those containing spaces and emit a warning instead of throwing, while
preserving valid headers in the generated command.
In `@studio/src/lib/playground-headers.ts`:
- Around line 9-25: Update substituteHeadersFromEnv to parse playground:env
inside a try block, returning an empty environment when parsing fails or yields
null/non-object data. Replace any with unknown for parsed environment values,
and narrow each value before boolean, numeric, or string conversion while
preserving existing header substitution behavior.
---
Nitpick comments:
In `@studio/src/__tests__/playground-curl.test.ts`:
- Around line 17-19: Update the expected command construction in the playground
curl test to use a single template literal for the complete curl command instead
of concatenating multiple template literals. Preserve the existing URL, headers,
line breaks, and serialized query payload.
In `@studio/src/components/playground/copy-operation.tsx`:
- Line 16: Update the CopyOperation component declaration to explicitly return
the applicable React JSX element type, while preserving its existing
implementation and behavior.
In `@studio/src/lib/playground-curl.ts`:
- Around line 26-81: Update parseJsonObject and related parsed JSON declarations
to use Record<string, unknown> instead of any. Narrow unknown header values
before string conversion, and ensure variables and request-body construction
handle unknown values without unchecked typing while preserving the existing
behavior.
In `@studio/src/lib/playground-headers.ts`:
- Around line 1-9: Add explicit return types to the exported helpers
validateHeaders and substituteHeadersFromEnv: declare validateHeaders as
returning void and substituteHeadersFromEnv as returning Record<string, string>,
preserving their existing implementations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 112099ff-2292-44f9-9238-f3ee1c46a992
📒 Files selected for processing (7)
studio/src/__tests__/playground-curl.test.tsstudio/src/components/playground/copy-operation.tsxstudio/src/components/playground/types.tsstudio/src/lib/playground-curl.tsstudio/src/lib/playground-headers.tsstudio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/playground.tsxstudio/src/styles/playground.css
| export const substituteHeadersFromEnv = (headers: Record<string, string>, graphId: string) => { | ||
| const env = JSON.parse(localStorage.getItem('playground:env') || '{}'); | ||
| const graphEnv: Record<string, any> | undefined = env[graphId]; | ||
|
|
||
| if (!graphEnv) { | ||
| return headers; | ||
| } | ||
|
|
||
| const storedHeaders: Record<string, any> = {}; | ||
|
|
||
| Object.entries(graphEnv).forEach(([key, value]) => { | ||
| if (value === 'true' || value === 'false') { | ||
| storedHeaders[key] = value === 'true'; | ||
| } else if (!isNaN(value as any) && value !== '') { | ||
| storedHeaders[key] = Number(value); | ||
| } else { | ||
| storedHeaders[key] = value; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle invalid stored environment data.
JSON.parse() throws when playground:env contains malformed JSON. It also permits null and non-object JSON values. The request path returns a network error, but CopyOperation does not catch this failure.
Parse the storage value in a try block. Return an empty environment when parsing fails or when the value is not an object. Replace any with unknown and narrow each environment value before conversion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@studio/src/lib/playground-headers.ts` around lines 9 - 25, Update
substituteHeadersFromEnv to parse playground:env inside a try block, returning
an empty environment when parsing fails or yields null/non-object data. Replace
any with unknown for parsed environment values, and narrow each value before
boolean, numeric, or string conversion while preserving existing header
substitution behavior.
Source: Coding guidelines
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (45.04%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3141 +/- ##
==========================================
- Coverage 62.37% 1.80% -60.58%
==========================================
Files 262 330 +68
Lines 31003 45970 +14967
Branches 0 516 +516
==========================================
- Hits 19337 828 -18509
- Misses 10158 44827 +34669
+ Partials 1508 315 -1193
🚀 New features to boost your workflow:
|
| */ | ||
| const shellQuote = (value: string) => `'${value.split("'").join(`'\\''`)}'`; | ||
|
|
||
| const parseJsonObject = (value: string | null | undefined): Record<string, any> | undefined => { |
There was a problem hiding this comment.
How about using zod schema for parsing this? We already use it in studio for forms.
| /** The url the playground sends its operations to. */ | ||
| routingUrl: string; | ||
| /** Set when a feature flag is selected in the playground toolbar. */ | ||
| featureFlagName?: string; |
There was a problem hiding this comment.
Nitpick: I'd initialize it in PlaygroundContext as featureFlagName: undefined
| } | ||
|
|
||
| /* replaced by our own copy menu, which can also copy the operation as a curl request */ | ||
| .graphiql-toolbar button[aria-label^='Copy query'] { |
There was a problem hiding this comment.
Can't we apply hidden class directly instead of this override? Wouldn't be possible to pass it via prop or use some kind of theme? If not let's keep it as is.
| return; | ||
| } | ||
|
|
||
| const { command, warnings } = buildCurlCommand({ |
There was a problem hiding this comment.
I'd just extract this out to a variable wrapped with useMemo just to reduce the size of the useCallback.
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Checklist