Env vars page performance fix#3829
Conversation
The Environment Variables page presenter loaded the entire project
secret store via a prefix scan and decrypted every value on each
render — including secret values that are immediately masked in the
UI — then matched rows with nested O(N×M²) `.find()` lookups.
- Collect only the non-secret (environmentId, key) pairs and fetch
them with a targeted `key IN (...)` query; decrypt only those.
- Add `getSecretsByKeys` to the secret store and
`getVariableValuesForKeys` to the repository for this access path.
- Replace the nested `.find()` lookups with O(1) Map lookups keyed
by `${environmentId}:${key}`.
Cuts per-render decryption and server CPU for projects with many
variables and environments; secret values stay masked as before.
The page server-rendered every row (~13 KB of markup each), producing a tens-of-MB HTML document and mounting thousands of row components on hydration, which froze the browser for projects with many variables across many environments. - Server-render only the first 50 rows, hydrate those, then switch to @tanstack/react-virtual over the full dataset after mount via useLayoutEffect (server and first client render match — no hydration mismatch). - Virtualize with a spacer-row technique inside the existing <table> so column widths and the sticky header are preserved; extract a shared EnvironmentVariableTableRow used by both the SSR and virtual paths to avoid drift. - Seed useFuzzyFilter from the URL `search` param (controlled mode, matching the Tasks page) so filtering happens at SSR and deep links render the correct rows in the initial window. For ~11k rows the document drops from ~150 MB to ~5 MB with 50 SSR rows; the load freeze is gone.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📜 Recent review details⏰ 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). (11)
WalkthroughThis PR implements backend and frontend performance optimizations for the Environment Variables page. The changes add a bulk-secret retrieval API to the SecretStore, extend the EnvironmentVariablesRepository with a method to fetch variable values for specified keys only, introduce a shared 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![github-advanced-security[bot]](https://github.com/_gh/avatars/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://github.com/_gh/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
apps/webapp/app/services/secrets/secretStore.server.ts (1)
144-174: ⚡ Quick winBatch parsing aborts entirely on a single schema-validation failure.
Invalid JSON is handled gracefully (log +
undefined), butschema.parseat Line 149 and Line 173 throws. Because#parseStoredSecretscalls this per secret, one malformed/legacy entry now causesgetSecretsByKeys/getSecretsto reject for the whole batch — which can break the entire Environment Variables SSR page rather than dropping one row. Consider making the batch path tolerant (skip + log) while keeping the single-secretgetSecretpath strict.Please confirm whether a single corrupt secret should fail the whole page load; if strict behavior is intended, ignore this.♻️ One option: tolerant batch wrapper
for (const secret of secrets) { - const value = await this.#parseStoredSecret(schema, secret); - if (value !== undefined) { - results.push({ key: secret.key, value }); - } + try { + const value = await this.#parseStoredSecret(schema, secret); + if (value !== undefined) { + results.push({ key: secret.key, value }); + } + } catch (error) { + logger.error(`Failed to parse secret ${secret.key}`, { error }); + } }apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (1)
701-705: ⚡ Quick winAdd
keyprops to spacer rows.The spacer
<tr>elements should have explicitkeyprops to prevent React warnings during re-renders.🔧 Suggested fix
{topSpacerHeight > 0 && ( - <tr aria-hidden style={{ height: topSpacerHeight }}> + <tr key="top-spacer" aria-hidden style={{ height: topSpacerHeight }}> <td colSpan={columnCount} /> </tr> )} {virtualItems.map((virtualRow) => { const variable = groupedEnvironmentVariables[virtualRow.index]; if (!variable) { return null; } return ( <EnvironmentVariableTableRow key={`${variable.id}-${variable.environment.id}`} variable={variable} revealAll={revealAll} vercelIntegration={vercelIntegration} /> ); })} {bottomSpacerHeight > 0 && ( - <tr aria-hidden style={{ height: bottomSpacerHeight }}> + <tr key="bottom-spacer" aria-hidden style={{ height: bottomSpacerHeight }}> <td colSpan={columnCount} /> </tr> )}Also applies to: 721-725
apps/webapp/test/environmentVariablesRepository.test.ts (1)
129-136: 💤 Low valueReuse the
uniqueIdhelper instead of rawDate.now()for slug/externalRef.This inline creation duplicates the fixture's project-creation logic and relies on
Date.now()alone for uniqueness, which is weaker than theuniqueIdhelper (counter + timestamp) used elsewhere. Prefer the shared helper for consistency and collision resistance.♻️ Suggested change
- data: { - name: "Project B", - slug: `proj-b-${Date.now()}`, - organizationId: organization.id, - externalRef: `ext-b-${Date.now()}`, - }, + data: { + name: "Project B", + slug: uniqueId("proj-b"), + organizationId: organization.id, + externalRef: uniqueId("ext-b"), + },Add
uniqueIdto the fixtures import on line 24-28.apps/webapp/app/presenters/v3/EnvironmentVariablesPresenter.server.ts (1)
123-131: ⚡ Quick winResidual nested
.findlookups undercut the stated O(1) goal.
nonSecretItems(here) and the finalflatMap(Line 154) both iterateenvironmentVariables × sortedEnvironmentsand call.findovervalueson each pair. The PR aims to replace nested.find()with O(1) Map lookups; consider building aMap<environmentId, valueRecord>perenvironmentVariableonce and reusing it in both passes to keep the hot path linear.♻️ Sketch
- const nonSecretItems: Array<{ environmentId: string; key: string }> = []; - for (const environmentVariable of environmentVariables) { - for (const env of sortedEnvironments) { - const valueRecord = environmentVariable.values.find((v) => v.environmentId === env.id); - if (valueRecord && !valueRecord.isSecret) { - nonSecretItems.push({ environmentId: env.id, key: environmentVariable.key }); - } - } - } + const valuesByEnvForVariable = new Map( + environmentVariables.map((ev) => [ + ev.id, + new Map(ev.values.map((v) => [v.environmentId, v])), + ]) + ); + const nonSecretItems: Array<{ environmentId: string; key: string }> = []; + for (const environmentVariable of environmentVariables) { + const byEnv = valuesByEnvForVariable.get(environmentVariable.id)!; + for (const env of sortedEnvironments) { + const valueRecord = byEnv.get(env.id); + if (valueRecord && !valueRecord.isSecret) { + nonSecretItems.push({ environmentId: env.id, key: environmentVariable.key }); + } + } + }Then reuse
valuesByEnvForVariablein the finalflatMapinstead of.find.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 92f33967-95f9-4d5b-8b90-7f0d6902b836
📒 Files selected for processing (18)
.server-changes/environment-variables-page-performance.mdapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/v3/CreateEnvironmentVariablesPresenter.server.tsapps/webapp/app/presenters/v3/EnvironmentVariablesPresenter.server.tsapps/webapp/app/presenters/v3/environmentVariablesEnvironments.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/services/secrets/secretStore.server.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.tsapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/scripts/measure-create-environment-variables-loader.mtsapps/webapp/scripts/measure-environment-variables-html.mtsapps/webapp/scripts/measure-environment-variables-new-parent-loader.mtsapps/webapp/scripts/spike-environment-variables-table-dom.mtsapps/webapp/test/EnvironmentVariablesPresenter.test.tsapps/webapp/test/environmentVariablesEnvironments.test.tsapps/webapp/test/environmentVariablesRepository.test.tsapps/webapp/test/fixtures/environmentVariablesFixtures.ts
kathiekiwi
force-pushed
the
env-vars-page-performance-fix
branch
from
74b25d6 to
3142404
Compare
Opening /environment-variables/new ran the full list presenter twice — once in the parent route loader and once in the child loader — fetching and decrypting every variable value just to show the create form, which only needs the list of environments. - Short-circuit the parent route loader on the /new path so it skips the list presenter entirely and renders only the create outlet. - Load just the environment list in the child route via a new CreateEnvironmentVariablesPresenter. - Extract the shared environment-loading logic into loadEnvironmentVariablesEnvironments, preserving the project access check and environment filtering for both presenters. Removes the heavy presenter work (full fetch + decrypt) when opening the create form.
kathiekiwi
force-pushed
the
env-vars-page-performance-fix
branch
from
3142404 to
8256c34
Compare
2 participants
Summary
This PR improves performance across the Environment Variables page.
Changes
Targeted value loading
SSR windowing + virtualization
?search=DATABASE_URLLightweight 'Create' flow
Results
Large projects no longer render thousands of rows during SSR.
Example (~11k rendered rows):
Testing
Automated
Manual