fix(code-help): make the SDK snippets runnable - #8145
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe change passes onboarding feature names directly into SDK snippet generation and adds a dedicated React Native generator. Language templates now initialise clients, retrieve environment flags, and display primary and alternate feature values using consistent fallback rules. React and Next.js templates conditionally subscribe to alternate flags. Onboarding connection state now carries an SDK label through the UI, with Storybook and end-to-end coverage. Jest transforms JavaScript templates, installation commands are updated, and highlighted-code styling gains tab sizing. Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
003d340 to
112bf20
Compare
112bf20 to
cfbb0b1
Compare
cfbb0b1 to
a857271
Compare
a857271 to
466a87f
Compare
466a87f to
56b7fcf
Compare
Python snippet was failing on copy-paste, and the same class of bug
affected most of the others. Each snippet was rendered and read back to confirm
the output.
- Python called json.loads with no import, and would have raised JSONDecodeError
on a plain string value anyway. It now reads the value directly.
- PHP read $flags without ever defining it.
- Ruby used // for comments, .NET used #.
- Go passed the environment key in single quotes, which is a rune literal.
- JavaScript called myCoolFeature(), which does not exist in the user's app.
- Next.js was missing a closing paren in the App signature, and both routers
ended in `<>{...}</>`, which does not compile.
- iOS, and 10 traits/create-user templates, used `cond && \`...\`` inside a
template literal, rendering the string "false" into the snippet whenever the
URL is not custom, i.e. on SaaS.
The second flag is now optional. codeHelp.INIT takes a feature name, and the
onboarding passes the flag it just created so the snippet references that one
flag rather than a placeholder nobody has. The Features page passes nothing and
keeps both placeholders as before. This also lets the onboarding drop its
string-replacement of the placeholder name.
Snippets no longer interpolate the flag name into variable names or dot access,
since the onboarding name is user-chosen and a space or hyphen made the JS
snippets a syntax error.
Adds a test over all 14 init snippets, covering both the one-flag and two-flag
shapes. It needed `.js` transforming in Jest, scoped so node_modules stays
untransformed; the full suite still runs in ~1s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2d1140a to
3bf6069
Compare
Docker builds report
|
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18826 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #18826 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18826 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18826 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18814 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18814 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18814 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Skipped testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss ✅ oss · depot-ubuntu-latest-16 — run #18814 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Actionable comments posted: 7
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 742c5ffe-76cf-49bb-a119-efdc2ca52f2b
📒 Files selected for processing (23)
frontend/common/code-help/__tests__/init-snippets.test.tsfrontend/common/code-help/init/init-dotnet.jsfrontend/common/code-help/init/init-flutter.jsfrontend/common/code-help/init/init-go.jsfrontend/common/code-help/init/init-ios.jsfrontend/common/code-help/init/init-java.jsfrontend/common/code-help/init/init-js.jsfrontend/common/code-help/init/init-next-app-router.jsfrontend/common/code-help/init/init-next-pages-router.jsfrontend/common/code-help/init/init-node.jsfrontend/common/code-help/init/init-php.jsfrontend/common/code-help/init/init-python.jsfrontend/common/code-help/init/init-react-native.jsfrontend/common/code-help/init/init-react.jsfrontend/common/code-help/init/init-ruby.jsfrontend/common/code-help/init/init-rust.jsfrontend/common/code-help/install/install-flutter.jsfrontend/common/code-help/install/install-php.jsfrontend/common/code-help/install/install-rust.jsfrontend/common/constants.tsfrontend/jest.config.jsfrontend/web/components/pages/onboarding/OnboardingConnectPanel/sdkSnippets.tsfrontend/web/styles/3rdParty/_hljs.scss
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1bcb1b73-a5fe-4e19-a732-c633bf9f2894
📒 Files selected for processing (5)
frontend/documentation/pages/onboarding/OnboardingTerminal.stories.tsxfrontend/e2e/tests/onboarding-tests.pw.tsfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsxfrontend/web/components/pages/onboarding/OnboardingTerminal/OnboardingTerminal.tsxfrontend/web/components/pages/onboarding/hooks/useOnboardingConnection.ts
|
@talissoncosta great job, I've gone through and run validation of every language against sample data and it all passes. I think that's as good as a review I can give, one thing's for sure is that it's an improvement - I think we should go ahead and merge this |
docs/if required so people know about the feature.Changes
Follow-up to #8132. The onboarding's Python snippet failed on copy-paste, so I ran every snippet through its real toolchain. Most were broken:
staticon a top-level declaration, and the constructor takes aFlagsmithConfiguration), Go (rune-literal key,GetEnvironmentFlagsmissing itsctx, nopackage/main), Next.js (missing paren,<>{...}</>).json.loadswith no import), Node (imports a default export the SDK lacks), PHP (undefined$flags, no<?php, install omitted the required PSR-18 client), Flutter (undefinedconfig,baseURIpassed to the wrong class), React Native (rendered<div>/<p>, which don't exist in RN), JavaScript (called a function the user doesn't have).//, .NET with#, iOS printed the literal stringfalse. The Rust and Flutter installs were prose, not commands.Snippets now print their result, and code blocks get
tab-sizeso Go's tabs don't render 8 columns wide.codeHelp.INITtakes an optional feature name. The onboarding passes the flag it created, so the snippet stops referencingbanner_size, a flag nobody has; the Features page passes nothing and keeps both placeholders as today. Flag names are no longer interpolated into variable names, since a user-chosen name with a space or hyphen broke the JS snippets.Identity and segment snippets are in #8146.
How did you test this code?
A test covers all 15 init snippets in both shapes (368 unit tests). It needed
.jsin the Jest transform, withnode_modulesexcluded again; the suite still runs in ~1s.Then each snippet through its toolchain, in Docker where needed:
go build,go run, gofmt-cleandotnet buildsucceeded, ranmvn compilesucceededcargo build, no warningsphp -l, composer, executeddart analyze, no issuesruby -conly, no gem installedAll runs used a placeholder key, so this shows the code is valid and the SDK calls exist, not that a real flag value came back. Java, Rust and Flutter are fragments, compiled inside a minimal wrapper; Go and .NET are now complete programs because their compilers reject fragments.