feat: swagger editor - #70
Conversation
* feat: install next+ts+linter+prettier+ husky+redux * feat: configs + perfectionist + lint fix * feat: install scss * refactor: add some folders * feat: install i18n + basic configs * feat: add internatiolisation * feat: language switcher * feat: dependencies update * feat: add sticky header * feat: add mui Button-and-Link-like component * fix: lint * style: add style for header * refactor: remove css from language selector * refactor: add const for flex styles in the header boxes * refactor: history style as link * fix: add some fixes
* chore: setup libraries * feat: swagger-editor * refactor: move title and description into a separated component * refactor: moved controls into the swaggerControl component * chore: update package-lock.json * fix: fix import in swagger-view * feat: add translations into SwaggerControl, SwaggerEditor, SwaggerViewer * fix: fix title translation in main text * fix: enforce unique keys when parsing YAML * refactor: extract DEFAULT_SCHEMA into a constant * feat: add tooltip explaining disabled format toggle * test: add test coverage for schema editing flow * fix: resolve component type errors in swagger viewer * fix: add request timeout with AbortController * fix: preserve Monaco editor undo history and cursor position on format switch * fix: prevent restore fetch from overwriting user-edited schema * fix: handle rejected promise in restoreSchema to prevent unhandled rejection * fix: replace missing setEditorValue with handleEditorChange in SwaggerEditor * fix: apply syntax highlighting immediately on editor mount * fix: preserve original JSON parse error * fix: disable external $ref resolution in schema validation to prevent SSRF * refactor: split SwaggerControl into service, component, and helper move save request to schema-service extract FormatToggle component extract getSignalWithTimeout helper switch errors to toast notifications * refactor: rename useSwaggerHook and drop unnecessary useMemo * test: fix tests after refactor * refactor: extract duplicated language-setting logic into helper in SwaggerMonacoEditor * refactor: remove redundant SwaggerViewer title * fix: use translated conversionError message instead of hardcoded string * refactor: extract SchemaFormat and isRecord into shared schema-types module * fix: reserve space for status alert to prevent layout shift on locale switch * feat: sync Monaco editor theme with app color scheme * style: add shadow, border-radius, deduplicate panel styles * test: fix ReactElement import in swagger-control tests * feat: extract schema fetch logic into SchemaService with restore and save methods * fix: extract magic number into named constant * refactor: extract isRecord type guard into its own module * fix: validate restore payload and cancel restore request on unmount * test: add tests for monaco theme * feat: add SwaggerViewer placeholder component * fix: route SwaggerControl toast messages through shared toaster namespace --------- Co-authored-by: Yulia Demir <Nightelfin91@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughIntroduces a client-side Swagger/OpenAPI editor with schema parsing, conversion (JSON/YAML), and validation utilities, a ChangesSwagger Editor Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SwaggerEditor
participant useSwagger
participant SchemaService
participant API
SwaggerEditor->>useSwagger: mount
useSwagger->>SchemaService: restore(signal)
SchemaService->>API: GET /api/schema
API-->>SchemaService: schema payload
SchemaService-->>useSwagger: RestoredSchema or null
useSwagger->>useSwagger: validateSwaggerSchema(editorValue)
useSwagger-->>SwaggerEditor: editorValue, isValid, schema, error
sequenceDiagram
participant SwaggerControl
participant SchemaService
participant API
participant ToastProvider
SwaggerControl->>SchemaService: save(content, format, signal)
SchemaService->>API: POST /api/schema
alt success
API-->>SchemaService: 200 OK
SchemaService-->>SwaggerControl: resolved
SwaggerControl->>ToastProvider: show success toast
else failure or timeout
API-->>SchemaService: error / aborted
SchemaService-->>SwaggerControl: throws Error
SwaggerControl->>ToastProvider: show error toast
end
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/utils/hooks/use-swagger.ts (1)
21-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded auth stub — tracked by TODO.
isAuthenticatedis permanentlytrue, so restore logic always runs regardless of actual auth state. This is flagged with a TODO, so likely tracked separately, but worth confirming it's on the backlog before merge to main.Want me to open a follow-up issue to track wiring real authentication into this hook?
🤖 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 `@src/utils/hooks/use-swagger.ts` around lines 21 - 24, The useSwagger hook currently hardcodes isAuthenticated to true, so the restore logic always runs regardless of the real auth state. Replace this stub in use-swagger.ts with the actual authentication check used by the app, keeping the logic inside useSwagger tied to the real auth source instead of a temporary TODO. If the real auth integration is not ready, remove the stubbed behavior and gate the restore path behind the existing auth state mechanism or confirm the follow-up is tracked separately.src/components/swagger-control/swagger-control.tsx (2)
36-58: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo unmount guard around the async save flow.
If the user navigates away or the component unmounts while
SchemaService.saveis pending, thefinallyblock still callssetIsSaving/cleanup, and a resolved/rejected promise still triggersshowToast, against a component/context that may no longer be mounted. Consider aborting on unmount (e.g. callcleanup()from auseEffectcleanup) or guarding state updates with anisMountedref.♻️ Suggested fix
+ useEffect(() => { + return () => { + // abort any in-flight save on unmount + }; + }, []);Alternatively, track the
AbortController/signalreturned bygetSignalWithTimeoutin a ref and call itscleanup()from auseEffectunmount callback so the fetch is aborted rather than left to resolve into an unmounted tree.🤖 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 `@src/components/swagger-control/swagger-control.tsx` around lines 36 - 58, The async save flow in handleSaveSchema lacks an unmount safeguard, so SchemaService.save can still resolve/reject after the component is gone. Update the SwaggerControl save path to abort or clean up on unmount by wiring getSignalWithTimeout/SAVE_TIMEOUT_MS into a useEffect cleanup, or guard showToast and setIsSaving with an isMounted ref so handleSaveSchema does not touch unmounted state.
29-32: 📐 Maintainability & Code Quality | 🔵 TrivialPlaceholder auth bypass with TODO.
isAuthenticatedis hardcoded totrue, guarded only by a TODO. This effectively disables the save-authorization gate for now — please make sure a tracking issue exists before merge.Want me to open a tracking issue for wiring up real authentication here?
🤖 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 `@src/components/swagger-control/swagger-control.tsx` around lines 29 - 32, The auth check in swagger-control is currently bypassed by hardcoding isAuthenticated to true, so replace this placeholder in swagger-control with the real authorization flow or remove the bypass if not needed; if the real auth wiring stays deferred, make sure there is a tracking issue linked to the TODO and referenced from the relevant component logic before merge.src/components/swagger-control/swagger-control.test.tsx (1)
25-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShared
vi.fn()mock isn't cleared between tests.
handleFormatToggleis a singlevi.fn()defined once at module scope and reused by every test.vi.restoreAllMocks()only restoresvi.spyOnoriginals — it does not reset call history for plainvi.fn()instances. This works today because only one test clicks the switch, but call counts will silently accumulate if another test later also triggers the toggle.♻️ Suggested fix
-const baseProps = { - editorValue: 'openapi: 3.0.0', - format: 'yaml' as const, - handleFormatToggle: vi.fn(), -}; +function createBaseProps() { + return { + editorValue: 'openapi: 3.0.0', + format: 'yaml' as const, + handleFormatToggle: vi.fn(), + }; +}Then call
createBaseProps()fresh in eachit(...), or addvi.clearAllMocks()/reassign inbeforeEach.Also applies to: 54-60
🤖 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 `@src/components/swagger-control/swagger-control.test.tsx` around lines 25 - 29, The shared vi.fn() for handleFormatToggle in swagger-control.test.tsx is defined once at module scope and its call history can leak across tests. Update the test setup around baseProps/createBaseProps so each test gets a fresh mock instance, or clear/reset the mock in beforeEach alongside the existing vi.restoreAllMocks() handling. Keep the fix localized to the swagger-control.test.tsx tests and ensure any test using the format toggle starts from a clean mock state.src/components/swagger-editor/swagger-editor.tsx (2)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSelf-referential barrel import creates a circular dependency.
src/components/index.tsre-exportsSwaggerEditorfrom this very file, so importingSwaggerMonacoEditor/SwaggerViewerfrom@componentshere forms a cycle:index.ts → swagger-editor.tsx → index.ts. It's also inconsistent withMainText/SwaggerControl, which are imported via relative paths right below. This is also why the test file has to mock the entire@componentsmodule instead of importing the concrete components directly.♻️ Suggested fix
-import { SwaggerMonacoEditor, SwaggerViewer } from '`@components`'; import { Alert, alpha, Box, Paper, Stack } from '`@mui/material`'; import { useTranslations } from 'next-intl'; import { useSwagger } from '`@/utils/hooks/use-swagger`'; import { getSwaggerEndpoints } from '`@/utils/swagger-editor/get-swagger-endpoints`'; import { MainText } from '../main-text/main-text'; import { SwaggerControl } from '../swagger-control/swagger-control'; +import { SwaggerMonacoEditor } from '../swagger-monaco-editor/swagger-monaco-editor'; +import { SwaggerViewer } from '../swagger-viewer/swagger-viewer';Also applies to: 12-13
🤖 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 `@src/components/swagger-editor/swagger-editor.tsx` at line 5, Avoid the circular dependency caused by importing SwaggerMonacoEditor and SwaggerViewer from the `@components` barrel inside SwaggerEditor. Update SwaggerEditor to import those components directly via relative paths, consistent with MainText and SwaggerControl, so src/components/index.ts no longer loops back through this file. Keep the existing exports in the barrel unchanged; only change the import sources in swagger-editor.tsx and adjust the related test setup if it can now import the concrete components instead of mocking the whole `@components` module.
56-60: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize
getSwaggerEndpoints(schema).This recomputes on every render of
SwaggerEditor, including re-renders triggered byeditorValue/formatstate changes unrelated toschema. For large specs — explicitly called out as a manual-check concern for this feature — recomputing the full endpoint walk on unrelated re-renders is wasteful and also produces a new array reference each time, causingSwaggerViewerto re-render needlessly.⚡ Suggested fix
+import { useMemo } from 'react'; ... - const endpoints = getSwaggerEndpoints(schema); + const endpoints = useMemo(() => getSwaggerEndpoints(schema), [schema]);🤖 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 `@src/components/swagger-editor/swagger-editor.tsx` around lines 56 - 60, Memoize the endpoint derivation in SwaggerEditor: the current getSwaggerEndpoints(schema) call is recomputed on every render and returns a new array reference, which needlessly re-renders SwaggerViewer. Update SwaggerEditor to cache the result based only on schema (for example by deriving endpoints inside a memoized value tied to schema) so editorValue/format changes do not retrigger the full schema walk.
🤖 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 `@src/components/index.ts`:
- Around line 4-9: The codebase currently has two SwaggerViewer implementations
with the same API, and the one in swagger-view appears stale because the app
uses the barrel-exported SwaggerViewer from the component index. Remove the
redundant swagger-view implementation and any unnecessary exports/tests tied
only to it, or consolidate it into the main SwaggerViewer component so there is
a single source of truth. Use the SwaggerViewer symbol and the component index
barrel exports to locate and clean up the duplicate.
In `@src/components/swagger-view/swagger-view.tsx`:
- Around line 1-66: There are two competing SwaggerViewer implementations with
the same SwaggerViewerProps and translations namespace, so consolidate to a
single source of truth. Check which component is actually used by
swagger-editor.tsx and the barrel export in src/components/index.ts, then either
remove the unused duplicate or merge the rendering logic into the intended
SwaggerViewer file. Make sure only one SwaggerViewer export remains to avoid
import ambiguity and dead code.
In `@src/i18n/messages/en.json`:
- Around line 28-33: The SwaggerControl translation key is mismatched with the
component usage, so the timeout toast text is not resolving from the expected
namespace. Reconcile the `swaggerControl.timeoutError` entry in the messages
file with `SwaggerControl` in `swagger-control.tsx` by either moving the
component to use the `swaggerControl` namespace/key or removing/renaming the
unused key, and make sure the toast lookups (`requestTimeoutError`,
`schemaSaveSuccess`, `schemaSaveError`) point to the same defined message names.
In `@src/services/schema-service.ts`:
- Around line 30-32: The JSON handling in schema-service’s response parsing is
assuming `response.json()` always returns an object, so `data.schema` can throw
on null or non-object payloads. Update the async fetch flow in the
schema-service function that reads `response.json()` to validate the parsed
value before accessing `schema`, and return `null` for invalid shapes just like
the `!response.ok` path. Use the existing `isRestoredSchema` check as the guard
point and make sure it safely handles non-object JSON results.
In `@src/utils/hooks/use-swagger.ts`:
- Around line 29-43: The debounce validation effect in useSwagger currently
awaits validateSwaggerSchema inside the setTimeout callback without any error
handling, which can cause unhandled promise rejections. Update the useEffect
logic to wrap the async validation path in try/catch (or equivalent promise
handling) and surface failures through the existing setError state, keeping
behavior consistent with the restore effect and preventing silent runtime
warnings.
---
Nitpick comments:
In `@src/components/swagger-control/swagger-control.test.tsx`:
- Around line 25-29: The shared vi.fn() for handleFormatToggle in
swagger-control.test.tsx is defined once at module scope and its call history
can leak across tests. Update the test setup around baseProps/createBaseProps so
each test gets a fresh mock instance, or clear/reset the mock in beforeEach
alongside the existing vi.restoreAllMocks() handling. Keep the fix localized to
the swagger-control.test.tsx tests and ensure any test using the format toggle
starts from a clean mock state.
In `@src/components/swagger-control/swagger-control.tsx`:
- Around line 36-58: The async save flow in handleSaveSchema lacks an unmount
safeguard, so SchemaService.save can still resolve/reject after the component is
gone. Update the SwaggerControl save path to abort or clean up on unmount by
wiring getSignalWithTimeout/SAVE_TIMEOUT_MS into a useEffect cleanup, or guard
showToast and setIsSaving with an isMounted ref so handleSaveSchema does not
touch unmounted state.
- Around line 29-32: The auth check in swagger-control is currently bypassed by
hardcoding isAuthenticated to true, so replace this placeholder in
swagger-control with the real authorization flow or remove the bypass if not
needed; if the real auth wiring stays deferred, make sure there is a tracking
issue linked to the TODO and referenced from the relevant component logic before
merge.
In `@src/components/swagger-editor/swagger-editor.tsx`:
- Line 5: Avoid the circular dependency caused by importing SwaggerMonacoEditor
and SwaggerViewer from the `@components` barrel inside SwaggerEditor. Update
SwaggerEditor to import those components directly via relative paths, consistent
with MainText and SwaggerControl, so src/components/index.ts no longer loops
back through this file. Keep the existing exports in the barrel unchanged; only
change the import sources in swagger-editor.tsx and adjust the related test
setup if it can now import the concrete components instead of mocking the whole
`@components` module.
- Around line 56-60: Memoize the endpoint derivation in SwaggerEditor: the
current getSwaggerEndpoints(schema) call is recomputed on every render and
returns a new array reference, which needlessly re-renders SwaggerViewer. Update
SwaggerEditor to cache the result based only on schema (for example by deriving
endpoints inside a memoized value tied to schema) so editorValue/format changes
do not retrigger the full schema walk.
In `@src/utils/hooks/use-swagger.ts`:
- Around line 21-24: The useSwagger hook currently hardcodes isAuthenticated to
true, so the restore logic always runs regardless of the real auth state.
Replace this stub in use-swagger.ts with the actual authentication check used by
the app, keeping the logic inside useSwagger tied to the real auth source
instead of a temporary TODO. If the real auth integration is not ready, remove
the stubbed behavior and gate the restore path behind the existing auth state
mechanism or confirm the follow-up is tracked separately.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 452ec2e5-f91c-4daa-81e4-b48bd5865b3b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
package.jsonsrc/app/[locale]/page.tsxsrc/app/globals.csssrc/components/format-toggle/format-toggle.tsxsrc/components/header/header.tsxsrc/components/index.tssrc/components/main-text/main-text.test.tsxsrc/components/main-text/main-text.tsxsrc/components/swagger-control/swagger-control.test.tsxsrc/components/swagger-control/swagger-control.tsxsrc/components/swagger-editor/swagger-editor.test.tsxsrc/components/swagger-editor/swagger-editor.tsxsrc/components/swagger-monaco-editor/swagger-monaco-editor.test.tsxsrc/components/swagger-monaco-editor/swagger-monaco-editor.tsxsrc/components/swagger-view/swagger-view.test.tsxsrc/components/swagger-view/swagger-view.tsxsrc/components/swagger-viewer/swagger-viewer.tsxsrc/constants/default-schema.tssrc/constants/index.tssrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonsrc/services/schema-service.tssrc/types/index.tssrc/utils/hooks/use-swagger.test.tssrc/utils/hooks/use-swagger.tssrc/utils/network/get-signal-with-timeout.tssrc/utils/swagger-editor/get-swagger-endpoints.tssrc/utils/swagger-editor/is-record.tssrc/utils/swagger-editor/schema-format-convert.test.tssrc/utils/swagger-editor/schema-format-parse.test.tssrc/utils/swagger-editor/schema-format.tssrc/utils/swagger-editor/schema-types.tssrc/utils/swagger-editor/swagger-validation.test.tssrc/utils/swagger-editor/swagger-validation.tstsconfig.json
💤 Files with no reviewable changes (1)
- src/app/globals.css
Summary
This PR delivers the core Swagger Editor functionality - loading and editing OpenAPI/Swagger schemas in both JSON and YAML, with automatic format detection, format conversion, and validation. It also lays the groundwork for the Swagger Viewer and schema persistence, both still in progress.
Changes Made
Related issue
Closes #40 #41 #42 #43 #46
Feature
Feature 3: Swagger Editor
Acceptance criteria
Feature 1: App Header
Feature 2: Sign In / Sign Up
Feature 3: Swagger Editor
Feature 4: Swagger Viewer
Feature 5: History and Analytics
Feature 6: About Page
Feature 7: General Requirements
Feature 8: YouTube Video
Type of change
Checklist
any/@ts-ignoreScreenshots
Summary by CodeRabbit
New Features
Bug Fixes
Tests