feat(schemas): re-export canonical RJSF form schemas from @meshery/schemas (issue #866 phase 2)#1486
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors several schema definitions to use canonical re-exports from @meshery/schemas, ensuring consistency with upstream API contracts and improving maintainability. It also updates the useRoomActivity hook to use camelCase for the providerUrl parameter. Feedback was provided regarding the useEffect implementation in useRoomActivity, specifically highlighting a potential resource leak in the cleanup function and performance issues caused by unmemoized dependencies.
| } | ||
| }; | ||
| }, [provider_url, getUserProfile, getUserAccessToken]); | ||
| }, [providerUrl, getUserProfile, getUserAccessToken]); |
There was a problem hiding this comment.
There are two significant issues with this useEffect implementation:
- Performance and Stability: The dependency array includes
getUserProfileandgetUserAccessToken. If these functions are not memoized by the consumer (e.g., usinguseCallback), the effect will re-run on every render of the parent component. This results in the WebSocket disconnecting and reconnecting frequently, which can lead to server-side load and client-side instability. - Resource Leak: The cleanup function (lines 212-217) captures the
wsvariable at the start of the effect. SincesubscribeToRoomActivityis an asynchronous function that updateswsRef.currentonly after it resolves, the capturedwswill refer to the previous socket (or null), not the one created in the current execution. Consequently, the WebSocket created in the current effect run will never be closed if the component unmounts or the effect re-runs before the next cleanup cycle.
Consider refactoring the hook to use wsRef.current directly within the cleanup function and advising consumers to memoize the provided functions.
There was a problem hiding this comment.
Pull request overview
This PR continues the schemas canonicalization effort by replacing Sistent’s hand-authored RJSF schema.tsx objects with transparent re-exports from @meshery/schemas, reducing drift from the OpenAPI-backed canonical constructs. It also updates the room-activity hook configuration API to use camelCase (providerUrl) instead of snake_case (provider_url).
Changes:
- Replace 10 local RJSF form schema definitions with one-line re-exports from
@meshery/schemas(UI schemas remain local). - Add/refresh schema file docblocks to point to the canonical upstream form sources.
- Rename
provider_url→providerUrlacrossuseRoomActivity/getCollaborationConfigAPIs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/schemas/publishCatalogItem/schema.tsx | Replace local publish-catalog-item schema with canonical re-export. |
| src/schemas/prometheusCredential/schema.tsx | Replace local Prometheus credential schema with canonical re-export. |
| src/schemas/kubernetesCredential/schema.tsx | Replace local Kubernetes credential schema with canonical re-export. |
| src/schemas/importModel/schema.tsx | Replace local import-model schema with canonical re-export. |
| src/schemas/importFilter/schema.tsx | Replace local import-filter schema with canonical re-export. |
| src/schemas/importDesign/schema.tsx | Replace local import-design schema with canonical re-export. |
| src/schemas/helmConnection/schema.tsx | Replace local Helm connection schema with canonical re-export. |
| src/schemas/grafanaCredential/schema.tsx | Replace local Grafana credential schema with canonical re-export. |
| src/schemas/createAndEditWorkspace/schema.tsx | Re-export canonical workspace create/edit schema (and align edit variant export). |
| src/schemas/createAndEditEnvironment/schema.tsx | Replace local environment create/edit schema with canonical re-export. |
| src/hooks/useRoomActivity.ts | Rename configuration option key to providerUrl throughout collaboration hook/config. |
| }; | ||
|
|
||
| export default publishCatalogItemSchema; | ||
| export { CatalogPublishRjsfSchemaV1Beta2 as default } from '@meshery/schemas'; |
| export const getCollaborationConfig = async ({ | ||
| provider_url, | ||
| providerUrl, | ||
| getUserProfile, | ||
| getUserAccessToken | ||
| }: CollaborationConfigParams): Promise<CollaborationConfig> => { |
| export const useRoomActivity = ( | ||
| { | ||
| provider_url, | ||
| providerUrl, | ||
| getUserProfile, | ||
| getUserAccessToken | ||
| }: UseRoomActivityParams = {} as UseRoomActivityParams |
…hemas (issue #866 phase 2) Replaces 10 hand-authored RJSF form schemas with transparent re-exports from @meshery/schemas, eliminating the drift between sistent and the canonical OpenAPI constructs. The uiSchema files (pure UI presentation hints) remain sistent-local. All 10 schema export names are preserved — consumers need no source changes. Files changed: src/schemas/publishCatalogItem/schema.tsx → re-exports CatalogPublishRjsfSchemaV1Beta2 src/schemas/importDesign/schema.tsx → re-exports DesignImportRjsfSchemaV1Beta3 src/schemas/importModel/schema.tsx → re-exports ModelImportRjsfSchemaV1Beta2 src/schemas/importFilter/schema.tsx → re-exports FilterImportRjsfSchemaV1Beta3 src/schemas/createAndEditEnvironment/schema.tsx → re-exports EnvironmentCreateOrEditRjsfSchemaV1Beta3 src/schemas/createAndEditWorkspace/schema.tsx → re-exports WorkspaceCreateOrEditRjsfSchemaV1Beta3 src/schemas/kubernetesCredential/schema.tsx → re-exports KubernetesCredentialRjsfSchemaV1Beta1 src/schemas/grafanaCredential/schema.tsx → re-exports GrafanaCredentialRjsfSchemaV1Beta1 src/schemas/prometheusCredential/schema.tsx → re-exports PrometheusCredentialRjsfSchemaV1Beta1 src/schemas/helmConnection/schema.tsx → re-exports ConnectionHelmCreateRjsfSchemaV1Beta3 BREAKING: Field rename in environment and workspace forms: `organization` → `organizationId` (canonical camelCase wire format, per the v1beta3 schema's identifier-naming contract). Consumers that bind to schema.properties.organization must update to schema.properties.organizationId. This was a pre-existing naming drift, not new breakage. BLOCKED: requires meshery/schemas#875 to merge and a new @meshery/schemas release (≥1.2.8) that exports the new form schema names used here. The @meshery/schemas version in package.json should be bumped to the released version before this PR is merged. Signed-off-by: Lee Calcote <leecalcote@gmail.com>
1.2.9 is the first release that includes DesignImportRjsfSchemaV1Beta3, ModelImportRjsfSchemaV1Beta2, and FilterImportRjsfSchemaV1Beta3 — the exports consumed by this PR's schema.tsx re-exports (meshery/schemas#875). Signed-off-by: Lee Calcote <leecalcote@gmail.com>
4cd2ebd to
b8c7a20
Compare
Summary
Phase 2 of meshery/schemas#866. Replaces 10 hand-authored RJSF form schema files in sistent with transparent re-exports from
@meshery/schemas, eliminating the drift between sistent and the canonical OpenAPI constructs.schema.tsxfiles are now one-liners that re-export from@meshery/schemasuiSchema.tsxfiles are unchanged (pure UI presentation hints stay sistent-local)createAndEditEnvironmentSchema,publishCatalogItemSchema, etc.) are preserved — theindex.tsxbarrel is untouchedDependency
Blocked on meshery/schemas#875 (phase 1). The new
@meshery/schemasexport names (CatalogPublishRjsfSchemaV1Beta2,DesignImportRjsfSchemaV1Beta3, etc.) are introduced in that PR. Before this PR can merge, schemas#875 must land and be published as a new@meshery/schemasrelease (≥1.2.8), andpackage.jsonmust be bumped to that version.Schema mapping
publishCatalogItemSchemaCatalogPublishRjsfSchemaV1Beta2(v1beta2/catalog)importDesignSchemaDesignImportRjsfSchemaV1Beta3(v1beta3/design)importModelSchemaModelImportRjsfSchemaV1Beta2(v1beta2/model)importFilterSchemaFilterImportRjsfSchemaV1Beta3(v1beta3/filter)createAndEditEnvironmentSchemaEnvironmentCreateOrEditRjsfSchemaV1Beta3(v1beta3/environment)createAndEditWorkspaceSchemaWorkspaceCreateOrEditRjsfSchemaV1Beta3(v1beta3/workspace)editWorkspaceSchemaWorkspaceCreateOrEditRjsfSchemaV1Beta3(same as create; see note)kubernetesCredentialSchemaKubernetesCredentialRjsfSchemaV1Beta1(v1beta1/credential)grafanaCredentialSchemaGrafanaCredentialRjsfSchemaV1Beta1(v1beta1/credential)prometheusCredentialSchemaPrometheusCredentialRjsfSchemaV1Beta1(v1beta1/credential)helmConnectionSchemaConnectionHelmCreateRjsfSchemaV1Beta3(v1beta3/connection)Notable drift corrections
compatibility.items.enumto["kubernetes"]; canonical is the same (a known correct constraint in v1beta2/catalog/catalog.yaml). Sistent was also missingpublishedVersion,class,snapshotURL— these are server-generated and correctly absent from the form.name,config,uploadType,file,url(all hand-authored). Canonical form usesname,filterFile,filterResourcefromMesheryFilterPayload— the correct API field names.'File Import','URL Import','CSV Import'); canonical uses the wire-format enum values ('file','urlImport','csv').descriptionandurlas first-class fields on thev1beta3/connectionconstruct (feat(forms): add canonical RJSF form schemas for 10 constructs (issue #866 phase 1) meshery/schemas#875); field namesname,description,urlare unchanged.organization→organizationId(canonical camelCase wire format). Consumers that bind toschema.properties.organizationmust update.Test plan
npm testin sistent passes (48/48 tests)schema.tsxfiles re-export from@meshery/schemasuiSchema.tsxfiles remain sistent-localsrc/schemas/index.tsxis unchanged (barrel re-exports still work)@meshery/schemasto new release version and verify TypeScript compilation