feat: Improve admin interface to match new UI#631
Conversation
🦋 Changeset detectedLatest commit: c0491dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughIntroduces TypeScript template “simple replacements” with parsing, validation, and rendering metadata; propagates variable metadata through TS renderers; restructures admin layout to a sidebar/breadcrumbs system; overhauls admin CRUD edit generator to renderer-driven with route loaders/crumbs; adds a CRUD migration (nameFieldRef) and UI to select it; refactors CRUD web editors to modal dialogs; expands core React/UI components. Changes
Sequence Diagram(s)sequenceDiagram
participant Template as TS Template File
participant Extractor as extractTsTemplateVariables
participant Parser as parseSimpleReplacements
participant Replacer as applySimpleReplacements
participant Delims as Delimited Var Processor
participant Renderer as renderTsTemplateToTsCodeFragment
Template->>Extractor: content
Extractor->>Parser: parseSimpleReplacements(content)
Parser-->>Extractor: {content', replacements}
alt replacements exist
Extractor->>Replacer: applySimpleReplacements(content', replacements)
Replacer-->>Extractor: content''
else
Extractor-->>Extractor: content' (no-op)
end
Extractor->>Delims: process START/END/INLINE markers
Delims-->>Extractor: { contentProcessed, variables }
Extractor-->>Renderer: variables (with types)
Renderer-->>Renderer: insert replacement comments (metadata-enabled)
Renderer-->>Template: rendered code
sequenceDiagram
participant Builder as compileAdminCrudSection
participant Gen as admin-crud-edit.generator
participant Renderers as Generated Renderers
participant Files as Output Files
Builder->>Gen: { modelId, modelName, nameField, disableCreate }
Gen->>Renderers: schema.render(...)
Gen->>Renderers: editForm.render({ TPL_FORM_DATA_NAME, ... })
Gen->>Renderers: createPage.render({ TPL_ROUTE_PATH, loader crumb })
Gen->>Renderers: editPage.render({ TPL_USER_QUERY, TPL_CRUMB_EXPRESSION, ... })
Gen->>Renderers: route.render({ TPL_ROUTE, TPL_CRUMB })
Renderers-->>Files: schema.ts, edit-form.tsx, create.tsx, edit.tsx, route.tsx
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
🔭 Outside diff range comments (8)
.changeset/template-replacements-feature.md (1)
6-6: Fix malformed changeset (stray character breaks parsing).There’s an extra literal “6” at Line 6 that will invalidate the changeset frontmatter parsing.
Apply this diff to remove it:
--- '@baseplate-dev/core-generators': patch --- Add template replacements support for TypeScript templates to enable dynamic value substitution in generated code -6packages/react-generators/src/generators/admin/admin-crud-edit/templates/create.tsx (1)
30-36: Add error handling around the create mutationCurrently, failures during the mutation will surface unhandled. Wrap in try/catch to provide a toast error and prevent navigating on failure.
Apply this diff:
- const submitData = async (formData: TPL_FORM_DATA_NAME): Promise<void> => { - await TPL_MUTATION_NAME({ - variables: { input: { data: formData } }, - }); - toast.success('Successfully created item!'); - navigate({ to: '..' }).catch(logError); - }; + const submitData = async (formData: TPL_FORM_DATA_NAME): Promise<void> => { + try { + await TPL_MUTATION_NAME({ + variables: { input: { data: formData } }, + }); + toast.success('Successfully created item!'); + await navigate({ to: '..' }); + } catch (err) { + logError(err); + toast.error('Failed to create item'); + } + };packages/react-generators/src/generators/admin/admin-crud-list/templates/Table.tsx (1)
1-1: Rename file to kebab-case: table.tsxOur convention requires kebab-case for filenames. Please rename the file and update references in the extractor accordingly.
packages/react-generators/src/generators/admin/admin-crud-list/extractor.json (1)
36-37: Keep extractor paths in sync if Table.tsx is renamedIf you rename the template to table.tsx, update these fields.
Apply this diff:
- "generatorTemplatePath": "Table.tsx", + "generatorTemplatePath": "table.tsx", @@ - "sourceFile": "Table.tsx", + "sourceFile": "table.tsx",Also applies to: 50-51
packages/core-generators/src/renderers/typescript/extractor/extract-ts-template-variables.ts (1)
63-71: Preserve variable metadata across phasesPhase 1 may set discoveredVariables[fullName] with type: 'replacement'. Phase 2 overwrites it with {} unconditionally, losing metadata.
Apply this diff to avoid clobbering existing entries:
- const processVariableBlock = ( + const processVariableBlock = ( varName: string, formatName: (name: string) => string, ): string => { const fullName = `TPL_${varName}`; - discoveredVariables[fullName] = {}; + if (!discoveredVariables[fullName]) { + discoveredVariables[fullName] = {}; + } return formatName(fullName); };packages/react-generators/src/generators/admin/admin-layout/templates/components/layouts/admin-layout.tsx (1)
1-1: Remove // @ts-nocheck to comply with strict typingThis template has proper typing on the component. Let’s keep type safety enabled.
-// @ts-nocheckpackages/project-builder-lib/src/schema/apps/web/admin/sections/crud-form/admin-crud-input.ts (1)
1-7: Use @src alias, add explicit external import, and group/sort imports per guidelines
- Replace non-standard alias
#src/with the configured@src/alias.- Add the external import of zod and place it before local imports.
- Keep external libraries first, then local imports.
Apply this diff:
-import { definitionSchema } from '#src/schema/creator/schema-creator.js'; - -import type { AdminCrudInputDefinition } from './types.js'; - -import { adminCrudInputSpec } from './admin-input-spec.js'; -import { adminCrudInputEntityType, baseAdminCrudInputSchema } from './types.js'; +import type { z } from 'zod'; + +import { definitionSchema } from '@src/schema/creator/schema-creator.js'; + +import type { AdminCrudInputDefinition } from './types.js'; +import { adminCrudInputSpec } from './admin-input-spec.js'; +import { adminCrudInputEntityType, baseAdminCrudInputSchema } from './types.js';packages/react-generators/src/generators/admin/admin-crud-edit/templates/edit.tsx (1)
35-41: Handle mutation errors and navigate deterministicallyWrap the mutation in try/catch and await navigation. Prefer
console.erroror the provided logger for visibility and a toast for UX.Apply this diff:
- const submitData = async (formData: TPL_FORM_DATA_NAME): Promise<void> => { - await TPL_MUTATION_NAME({ - variables: { input: { id, data: formData } }, - }); - toast.success('Successfully updated item!'); - navigate({ to: '..' }).catch(logError); - }; + const submitData = async (formData: TPL_FORM_DATA_NAME): Promise<void> => { + try { + await TPL_MUTATION_NAME({ + variables: { input: { id, data: formData } }, + }); + toast.success('Successfully updated item!'); + await navigate({ to: '..' }); + } catch (err) { + toast.error('Sorry we could not update the item.'); + logError(err); + } + };
🧹 Nitpick comments (44)
plugins/plugin-storage/src/storage/admin-crud/admin-crud-input-form.tsx (1)
23-25: Type-safe Control generic looks good; consider removing the cast if props support generics.The cast works, but if
AdminCrudInputWebFormPropssupports a generic parameter, prefer typing the component props to avoid theas Control<...>cast. It reduces the risk of drift between form props and the control’s type.If supported by the lib, consider this pattern:
// Adjust the prop type: export function AdminCrudFileInputForm({ formProps, name, model, }: AdminCrudInputWebFormProps<AdminCrudFileInputInput>): React.JSX.Element { // Then you can drop the cast: // const controlTyped = formProps.control; }plugins/plugin-storage/src/storage/admin-crud/types.ts (1)
13-18: Clarify the semantics of modelRelationRef.The field name suggests a relation reference, but in practice it references a “file transformer” entity (then resolves its
fileRelationRef). Consider a brief inline doc to avoid confusion for future readers.Apply this inline comment:
baseAdminCrudInputSchema.extend({ type: z.literal('file'), - modelRelationRef: ctx.withRef({ + // Points to the File Transformer entity (not directly to a model relation). + modelRelationRef: ctx.withRef({ type: modelTransformerEntityType, onDelete: 'RESTRICT', parentPath: { context: 'model' }, }), }),plugins/plugin-storage/src/storage/admin-crud/web.ts (1)
16-22: Registration generic updated; consider tightening getNewInput with satisfies.The registration now targets the new input type—great. As a minor improvement, you can use
satisfiesto ensure the default object stays in sync withAdminCrudFileInputInputwithout over-widening the literal types.Here’s a minimal tweak:
- adminCrudInputWeb.registerInputWebConfig<AdminCrudFileInputInput>({ + adminCrudInputWeb.registerInputWebConfig<AdminCrudFileInputInput>({ name: 'file', pluginKey, label: 'File', - getNewInput: () => ({ label: '', type: 'file', modelRelationRef: '' }), + getNewInput: () => + ({ label: '', type: 'file', modelRelationRef: '' }) satisfies AdminCrudFileInputInput, Form: AdminCrudFileInputForm, });If your TS version doesn’t support
satisfies, feel free to skip this..changeset/dry-crabs-love.md (1)
5-5: Nit: Improve the changeset sentence for clarityThe sentence reads a bit abrupt. Suggest adding “the” and ending with a period.
-Update admin generators to use new admin layout with breadcrumbs +Update the admin generators to use the new admin layout with breadcrumbs.packages/project-builder-server/src/sync/template-metadata-utils.ts (1)
37-41: Behavior change: empty whitelist now enables metadata for all instance files — confirm intentReturning true when compiledPatterns is empty changes behavior to always generate metadata for instance files if the whitelist is empty. This aligns with the metadata-first rendering direction, but it’s worth confirming this won’t create unexpected metadata churn in large repos.
If this is the intended default, consider documenting it inline for future maintainers:
if (!context.isInstance) return true; - if (compiledPatterns.length === 0) return true; + // If no whitelist patterns are configured, default to generating metadata for all instance files. + if (compiledPatterns.length === 0) return true;Optionally, you may want to treat whitespace-only configuration equivalently at the source by guarding the split earlier (not required for this change).
packages/react-generators/src/generators/admin/admin-layout/templates/routes/route.tsx (2)
28-30: Crumb added via loader: consider making the label configurableHardcoding 'Dashboard' works as a sensible default, but making it token-driven enables localization or branding without patching the template later.
For example, introduce a template token fallback:
- loader: () => ({ - crumb: 'Dashboard', - }), + loader: () => ({ + // Prefer a template token if provided; fallback to "Dashboard" + crumb: TPL_CRUMB ?? 'Dashboard', + }),If a token isn’t desirable here, consider adding a brief comment that this is the root crumb label used by AppBreadcrumbs.
1-1: Avoid shipping @ts-nocheck in generated codeIf feasible, limit @ts-nocheck to the template pre-processing stage only. Shipping it into generated routes can hide real type errors (e.g., loader data types).
If the placeholders force ts-nocheck during templating, consider post-processing to strip it from final outputs and add explicit return types on loader/beforeLoad shapes.
packages/project-builder-lib/src/references/definition-ref-builder.ts (1)
401-407: Strengthen error context by including the entity idGreat addition of a clear runtime guard. Including the entity id will make debugging much faster when names are missing.
- throw new Error( - `No name resolver found for entity ${entity.type.name} at ${path.join( - '.', - )}`, - ); + throw new Error( + `No name resolver found for entity ${entity.type.name} (id: ${id}) at ${path.join('.')}`, + );Also consider updating the JSDoc on getNameResolver to mention that a missing value will trigger this error.
packages/core-generators/src/renderers/typescript/actions/render-ts-template-group-action.unit.test.ts (1)
14-15: Mock Node built-ins per guidelines to avoid resolver mismatchesPer the repo’s testing guidelines, prefer mocking 'node:fs' and 'node:fs/promises' to ensure consistency with Node 16 module resolution and avoid bundler alias pitfalls.
-vi.mock('fs'); -vi.mock('fs/promises'); +vi.mock('node:fs'); +vi.mock('node:fs/promises');packages/core-generators/src/renderers/typescript/templates/types.ts (1)
18-20: Default variable type to 'delimited' to simplify downstream codeMaking type optional forces consumers to handle undefined. Prefer a default so inferred metadata is always concrete.
Apply this diff:
-const tsTemplateFileVariableSchema = z.object({ - type: z.enum(['delimited', 'replacement']).optional(), -}); +const tsTemplateFileVariableSchema = z.object({ + type: z.enum(['delimited', 'replacement']).default('delimited'), +});packages/react-generators/src/generators/core/react-components/templates/components/ui/skeleton.tsx (1)
10-21: Consider marking the skeleton as aria-hidden to reduce screen reader noiseSkeletons are purely visual; hiding them from AT avoids redundant announcements. Consumers can override via props if needed.
Apply this diff:
return ( <div data-slot="skeleton" className={cn('animate-pulse rounded-md bg-accent', className)} + aria-hidden="true" {...props} /> );packages/project-builder-web/src/routes/admin-sections.$appKey/-components/field-dialog.tsx (2)
33-41: Export Props interface for external consumptionProps is part of this component’s surface and may be reused by callers; exporting it improves reusability and adheres to the guideline to export relevant types.
Apply this diff:
-interface Props { +export interface Props { open: boolean; onOpenChange: (open: boolean) => void; field?: AdminCrudInputInput; modelRef: string; embeddedFormOptions: { label: string; value: string }[]; isNew?: boolean; onSave: (field: AdminCrudInputDefinition) => void; }
43-51: Align component return type with guidelineUse React.ReactElement for top-level React component return types per coding guidelines.
Apply this diff:
-export function FieldDialog({ +export function FieldDialog({ open, onOpenChange, field, modelRef, embeddedFormOptions, isNew = false, onSave, -}: Props): React.JSX.Element { +}: Props): React.ReactElement {packages/core-generators/src/renderers/typescript/extractor/parse-simple-replacements.ts (1)
8-10: Clarify allowed value regexThe current char class works but is harder to read and reason about. A simpler equivalent improves maintainability and reduces mistakes.
Apply this diff:
-const ALLOWED_VALUE_PATTERN = /^[a-zA-Z0-9_$/./-]+$/; +const ALLOWED_VALUE_PATTERN = /^[\w$./-]+$/;This allows: A–Z, a–z, 0–9, underscore, dollar, dot, slash, and hyphen.
Also applies to: 102-104
packages/core-generators/src/renderers/typescript/actions/render-ts-template-file-action.unit.test.ts (1)
110-112: Mismatch case updated for new variable descriptors — LGTM.Covers the “missing provided variable” path. Consider adding a sibling test where an extra variable is provided but not declared in the template to assert bi-directional validation.
Example outline:
- Template declares only TPL_GREETING
- Provided variables: TPL_GREETING + TPL_EXTRA
- Expect mismatch error
packages/react-generators/src/generators/admin/admin-crud-edit/templates/route.tsx (1)
1-1: Scope down ts-nocheck to avoid masking unrelated issues.Understandable for template placeholders, but consider local @ts-expect-error at the placeholder sites with a brief comment, so other mistakes in this file won’t be ignored.
For example:
- Remove // @ts-nocheck
- Add targeted // @ts-expect-error before the lines referencing TPL_ROUTE / TPL_CRUMB with a short rationale
packages/project-builder-lib/src/schema/apps/web/admin/sections/types.ts (1)
9-14: New adminCrudSectionColumnEntityType — LGTM.Parent linkage to adminSectionEntityType is clear and consistent with the schema hierarchy.
If not already done, re-export this from the nearest sections index barrel for discoverability:
// packages/project-builder-lib/src/schema/apps/web/admin/sections/index.ts
export * from './types.js'packages/project-builder-lib/src/migrations/index.ts (1)
85-87: Optional: Make latest migration lookup order-agnosticgetLatestMigrationVersion currently relies on array order. If someone inserts out of order later, this could drift. Consider deriving the max version instead.
Apply this diff:
-export function getLatestMigrationVersion(): number { - return SCHEMA_MIGRATIONS.at(-1)?.version ?? 0; -} +export function getLatestMigrationVersion(): number { + return Math.max(0, ...SCHEMA_MIGRATIONS.map((m) => m.version)); +}packages/react-generators/src/generators/admin/admin-layout/admin-layout.generator.ts (1)
84-92: Add active state props to sidebar links for accessibility and stylingProvide
aria-current="page"(and optionally a data attribute) when a link is active to improve a11y and enable styling of the current section.Apply this diff inside the Link:
- <Link to="${link.path}"> + <Link + to="${link.path}" + activeProps={{ 'aria-current': 'page', 'data-active': 'true' }} + >Optionally, ensure the SidebarMenuButton/Item styles react to
[data-active="true"].packages/project-builder-lib/src/migrations/migration-018-crud-name-field-ref.unit.test.ts (1)
300-337: Add an immutability test to prevent in-place mutation regressionsIt’s valuable to assert that migrate() returns a new config without mutating the input, especially for pipeline consumers relying on persistence or diffing.
Apply this diff near the end of the file (before the closing describe bracket):
+ it('does not mutate the input config (immutability)', () => { + const oldConfig = { + models: [ + { + id: 'user-model', + name: 'User', + model: { + fields: [ + { id: 'field-1', name: 'id' }, + { id: 'field-2', name: 'name' }, + ], + }, + }, + ], + apps: [ + { + id: 'web-app', + type: 'web', + name: 'Web App', + adminApp: { + enabled: true, + pathPrefix: '/admin', + sections: [ + { + id: 'section-1', + name: 'Users', + type: 'crud', + modelRef: 'User', + }, + ], + }, + }, + ], + }; + + const oldConfigClone = structuredClone(oldConfig); + const result = migration018CrudNameFieldRef.migrate(oldConfig); + + // Output differs structurally from input (migration applied) + expect(result).not.toEqual(oldConfig); + // Input remains unchanged + expect(oldConfig).toEqual(oldConfigClone); + });packages/react-generators/src/generators/admin/admin-layout/templates/components/layouts/app-breadcrumbs.tsx (1)
3-14: Sort and group imports per guidelinesExternal libs first (react, @TanStack), then UI components via import provider aliases, then local/alias utilities. Consider grouping:
- react, Fragment (type and value)
- @tanstack/react-router
- %reactComponentsImports
packages/react-generators/src/generators/admin/admin-crud-list/templates/Table.tsx (1)
17-21: Import order nitMove external imports (sonner, react-icons, @tanstack/react-router) above alias imports (like %reactErrorImports) to comply with the import ordering guideline.
packages/react-generators/src/generators/admin/admin-layout/templates/components/layouts/admin-layout.tsx (1)
5-13: Sort imports: external before localPer our import ordering rule, external libs should be listed before local/template imports. Move the Outlet import above the local ones.
-import { AppBreadcrumbs } from '$appBreadcrumbs'; -import { AppSidebar } from '$appSidebar'; -import { - Separator, - SidebarProvider, - SidebarTrigger, -} from '%reactComponentsImports'; -import { Outlet } from '@tanstack/react-router'; +import { Outlet } from '@tanstack/react-router'; +import { AppBreadcrumbs } from '$appBreadcrumbs'; +import { AppSidebar } from '$appSidebar'; +import { Separator, SidebarProvider, SidebarTrigger } from '%reactComponentsImports';packages/react-generators/src/generators/admin/admin-crud-edit/templates/edit.tsx (3)
3-9: Sort imports by group: external libs first, then localMove the local
%reactErrorImportsimport after external libraries to comply with our import order rules.Apply this diff:
-import type { ReactElement } from 'react'; - -import { logError } from '%reactErrorImports'; -import { useMutation } from '@apollo/client'; +import type { ReactElement } from 'react'; +import { useMutation } from '@apollo/client'; import { createFileRoute, useNavigate } from '@tanstack/react-router'; import { toast } from 'sonner'; +import { logError } from '%reactErrorImports';
12-21: Validate route params and tighten loader resilienceAdd a guard for missing
id. Optional: keep the query as-is to bubble errors to the route error boundary.Apply this diff:
loader: async ({ context: { apolloClient }, params }) => { - const { id } = params; + const { id } = params; + if (!id) { + throw new Error('Missing "id" route param'); + } const { data } = await apolloClient.query({ query: TPL_USER_QUERY, variables: { id }, }); return { crumb: TPL_CRUMB_EXPRESSION, }; },
1-1: Avoid global @ts-nocheck where feasibleTemplate placeholders often necessitate disabling types, but consider scoping suppressions (e.g., with inline
@ts-expect-errornear placeholders) to retain type safety for the rest of the file once tokens are resolved.packages/core-generators/src/renderers/typescript/extractor/apply-simple-replacements.unit.test.ts (2)
48-68: Align import specifiers in test fixtures with Node 16 ESM (.js extension)Our code-style requires explicit
.jsextensions for local imports. Update the test fixture strings accordingly to prevent drifting from the repository’s conventions.Apply this diff:
- import { UserDocument } from './user-document.ts'; -import type { UserName } from './user-name.ts'; + import { UserDocument } from './user-document.js'; +import type { UserName } from './user-name.js';
221-231: Add .js extensions in multi-line import fixtureStay consistent with Node 16 module resolution in test inputs as well.
Apply this diff:
import { UserDocument, UpdateUserDocument, DeleteUserDocument - } from './user-document'; + } from './user-document.js'; import type { UserType, UserFormData - } from './types'; + } from './types.js';packages/project-builder-web/src/routes/admin-sections.$appKey/-components/crud-table-columns-form.tsx (1)
43-47: Consider memoizing the initial state to avoid unnecessary re-renders.The
editingColumnstate is initialized withundefinedon every render. While not a performance issue in this case, it's a good practice to ensure consistency.packages/core-generators/src/renderers/typescript/extractor/parse-simple-replacements.unit.test.ts (1)
59-68: Quote the boolean key in the expected map for clarity and consistency.While
{ true: '...' }is valid (property name coerces to "true"), quoting the key improves readability and avoids confusion with boolean literals.- expect(result.replacements).toEqual({ + expect(result.replacements).toEqual({ UserEditPage: 'TPL_COMPONENT_NAME', '/admin/accounts/users/$id': 'TPL_ROUTE_PATH', 'user-edit-form': 'TPL_CSS_CLASS', '42': 'TPL_COUNT', - true: 'TPL_ENABLED', + 'true': 'TPL_ENABLED', './components/user-form': 'TPL_MODULE_PATH', '/api/v1/users.json': 'TPL_API_PATH', - }); + });packages/core-generators/src/renderers/typescript/renderers/template.ts (1)
98-109: Minor: else branch is effectively unreachable.
importEndPatternwill always match due to(.*)at the end;matchis never falsy. You can simplify by dropping theelsebranch.- if (match) { - const [, importsSection, restOfCode] = match; - return `${importsSection}\n${replacementComments.join('\n')}\n\n${restOfCode}`; - } else { - // No imports found, add at the beginning - return `${replacementComments.join('\n')}\n\n${renderedTemplate}`; - } + const [, importsSection, restOfCode] = match!; + return `${importsSection}\n${replacementComments.join('\n')}\n\n${restOfCode}`;packages/project-builder-lib/src/schema/apps/web/admin/sections/crud-form/built-in-input.ts (1)
20-31: Remove redundantlabelfields already defined inbaseAdminCrudInputSchema.
baseAdminCrudInputSchemaincludeslabel: z.string().min(1). Re-declaring it in each extended schema is unnecessary and increases maintenance overhead.export const createAdminCrudTextInputSchema = definitionSchema((ctx) => - baseAdminCrudInputSchema.extend({ + baseAdminCrudInputSchema.extend({ type: z.literal('text'), - label: z.string().min(1), modelFieldRef: ctx.withRef({ type: modelScalarFieldEntityType, onDelete: 'RESTRICT', parentPath: { context: 'model' }, }), validation: z.string().optional(), }), );export const createAdminCrudForeignInputSchema = definitionSchema((ctx) => - baseAdminCrudInputSchema.extend({ + baseAdminCrudInputSchema.extend({ type: z.literal('foreign'), - label: z.string().min(1), localRelationRef: ctx.withRef({ type: modelLocalRelationEntityType, onDelete: 'RESTRICT', parentPath: { context: 'model' }, }), labelExpression: z.string().min(1), valueExpression: z.string().min(1), defaultLabel: z.string().optional(), nullLabel: z.string().optional(), }), );export const createAdminCrudEnumInputSchema = definitionSchema((ctx) => - baseAdminCrudInputSchema.extend({ + baseAdminCrudInputSchema.extend({ type: z.literal('enum'), - label: z.string().min(1), modelFieldRef: ctx.withRef({ type: modelScalarFieldEntityType, onDelete: 'RESTRICT', parentPath: { context: 'model' }, }), }), );export const createAdminCrudEmbeddedInputSchema = definitionSchema((ctx) => - baseAdminCrudInputSchema.extend({ + baseAdminCrudInputSchema.extend({ type: z.literal('embedded'), - label: z.string().min(1), modelRelationRef: ctx.withRef({ type: modelForeignRelationEntityType, onDelete: 'RESTRICT', parentPath: { context: 'model' }, }), embeddedFormRef: ctx.withRef({ type: adminCrudEmbeddedFormEntityType, parentPath: { context: 'admin-section' }, onDelete: 'RESTRICT', }), }), );export const createAdminCrudEmbeddedLocalInputSchema = definitionSchema((ctx) => - baseAdminCrudInputSchema.extend({ + baseAdminCrudInputSchema.extend({ type: z.literal('embeddedLocal'), - label: z.string().min(1), localRelationRef: ctx.withRef({ type: modelLocalRelationEntityType, onDelete: 'RESTRICT', parentPath: { context: 'model' }, }), embeddedFormRef: ctx.withRef({ type: adminCrudEmbeddedFormEntityType, parentPath: { context: 'admin-section' }, onDelete: 'RESTRICT', }), }), );export const createAdminCrudPasswordInputSchema = definitionSchema(() => - baseAdminCrudInputSchema.extend({ + baseAdminCrudInputSchema.extend({ type: z.literal('password'), - label: z.string().min(1), }), );Also applies to: 42-56, 67-77, 88-104, 114-129, 140-145
packages/project-builder-web/src/routes/admin-sections.$appKey/-components/column-dialog.tsx (1)
52-61: Type the form for stronger safety and better editor support.Add a generic to
useFormto align the control/fields with the schema/types you’re editing.- const form = useForm({ + const form = useForm<AdminCrudTableColumnDefinition>({ resolver: zodResolver(columnSchema), values: column ?? { label: '', display: { type: 'text' as const, modelFieldRef: '', }, }, });packages/project-builder-web/src/routes/admin-sections.$appKey/-components/crud-form-fields-form.tsx (3)
128-132: Reset editingIndex when closing the dialog.Prevents stale indices leaking across sessions.
onOpenChange={(open) => { if (!open) { setIsEditing(false); setIsCreating(false); setEditingField(undefined); + setEditingIndex(null); } }}
134-139: Use nullish coalescing for defaulting modelRef.Guideline: prefer
??over||.- modelRef={useWatch({ control, name: 'modelRef' }) || ''} + modelRef={useWatch({ control, name: 'modelRef' }) ?? ''}
41-54: Optional: hoist useWatch calls for readability and consistent hook placement.Declare
const modelRef = useWatch(...)andconst formFields = useWatch(...)near the other hooks, then pass the variables. This clarifies dependencies and keeps hooks grouped.Also applies to: 134-139
packages/react-generators/src/generators/core/react-components/templates/components/ui/sheet.tsx (2)
8-11: Sort imports: external libraries first, then local utilitiesPer guidelines, place external imports (radix-ui, react-icons) before local aliases like $cn.
-import { cn } from '$cn'; -import { Dialog as SheetPrimitive } from 'radix-ui'; -import { MdClose } from 'react-icons/md'; +import { Dialog as SheetPrimitive } from 'radix-ui'; +import { MdClose } from 'react-icons/md'; +import { cn } from '$cn';
85-89: Reuse the exported SheetClose for consistency and testabilityYou defined a SheetClose wrapper that adds a data-slot. Inside Content you’re using the primitive directly, which skips the data-slot and centralizes less.
- <SheetPrimitive.Close className="absolute top-4 right-4 rounded-xs opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:ring-2 focus:ring-ring focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none data-[state=open]:bg-secondary"> + <SheetClose className="absolute top-4 right-4 rounded-xs opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:ring-2 focus:ring-ring focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none data-[state=open]:bg-secondary"> <MdClose className="size-4" /> <span className="sr-only">Close</span> - </SheetPrimitive.Close> + </SheetClose>packages/react-generators/src/generators/core/react-components/templates/components/ui/dropdown.tsx (2)
7-10: Sort imports: external libraries first, then local utilitiesMove radix-ui and react-icons above the local cn import to comply with import grouping.
-import { cn } from '$cn'; -import { DropdownMenu as DropdownMenuPrimitive } from 'radix-ui'; -import { MdCheck, MdChevronRight, MdCircle } from 'react-icons/md'; +import { DropdownMenu as DropdownMenuPrimitive } from 'radix-ui'; +import { MdCheck, MdChevronRight, MdCircle } from 'react-icons/md'; +import { cn } from '$cn';
94-99: Double-check Tailwind selector: data-[variant=destructive]:*:[svg]:!text-destructiveThe segment "*:[svg]" looks suspicious and may not be recognized by Tailwind unless you’ve added a custom variant/plugin for the universal selector. If the intent is to color any svg descendants when variant=destructive, consider a safer pattern.
Option if standard Tailwind:
- "relative flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-sm outline-hidden select-none focus:bg-accent focus:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50 data-[inset]:pl-8 data-[variant=destructive]:text-destructive data-[variant=destructive]:focus:bg-destructive/10 data-[variant=destructive]:focus:text-destructive dark:data-[variant=destructive]:focus:bg-destructive/20 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4 [&_svg:not([class*='text-'])]:text-muted-foreground data-[variant=destructive]:*:[svg]:!text-destructive", + "relative flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-sm outline-hidden select-none focus:bg-accent focus:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50 data-[inset]:pl-8 data-[variant=destructive]:text-destructive data-[variant=destructive]:focus:bg-destructive/10 data-[variant=destructive]:focus:text-destructive dark:data-[variant=destructive]:focus:bg-destructive/20 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4 [&_svg:not([class*='text-'])]:text-muted-foreground data-[variant=destructive] [&_svg]:text-destructive",If you indeed rely on a custom variant, ignore this suggestion; otherwise, the adjusted selector will work with stock Tailwind.
packages/ui-components/src/components/ui/sidebar/sidebar.tsx (2)
10-29: Import grouping: external libraries first, then local modulesReorder imports so external packages (class-variance-authority, radix-ui, react, react-icons) precede internal '#src/*' imports. This aligns with repo guidelines and improves readability.
-import { cva } from 'class-variance-authority'; -import { Slot } from 'radix-ui'; -import * as React from 'react'; -import { MdMenu } from 'react-icons/md'; - -import { Button } from '#src/components/ui/button/button.js'; -import { Input } from '#src/components/ui/input/input.js'; -import { Separator } from '#src/components/ui/separator/separator.js'; +import { cva } from 'class-variance-authority'; +import { Slot } from 'radix-ui'; +import * as React from 'react'; +import { MdMenu } from 'react-icons/md'; + +import { Button } from '#src/components/ui/button/button.js'; +import { Input } from '#src/components/ui/input/input.js'; +import { Separator } from '#src/components/ui/separator/separator.js';Note: If the project enforces '@src' as the alias (per guidelines), please confirm whether '#src' is the canonical alias here.
296-313: Verify Tailwind variant "in-data[...]": likely a typoThe classes "in-data-[side=left]:cursor-w-resize" and "in-data-[side=right]:cursor-e-resize" look off. Unless you have a custom "in-data" variant, these probably should be "data-[side=left]:..." or something similar.
If this is a typo, consider:
- 'in-data-[side=left]:cursor-w-resize in-data-[side=right]:cursor-e-resize', + 'data-[side=left]:cursor-w-resize data-[side=right]:cursor-e-resize',If you do have a custom "in-data" variant configured, ignore this suggestion.
packages/react-generators/src/generators/core/react-components/templates/components/ui/sidebar.tsx (2)
5-10: Import grouping: external modules before local template aliasesMove external imports (class-variance-authority, radix-ui, react, react-icons) above local $-aliases to follow import ordering guidelines.
-import type { VariantProps } from 'class-variance-authority'; - -import { Button } from '$button'; -import { cn } from '$cn'; -import { Input } from '$input'; -import { Separator } from '$separator'; +import type { VariantProps } from 'class-variance-authority'; +import { cva } from 'class-variance-authority'; +import { Slot } from 'radix-ui'; +import * as React from 'react'; +import { MdMenu } from 'react-icons/md'; + +import { Button } from '$button'; +import { cn } from '$cn'; +import { Input } from '$input'; +import { Separator } from '$separator';And remove the later duplicate external imports (cva, Slot, React, MdMenu) to avoid reordering confusion.
448-464: Align hover styles and intent with UI package variantYour UI-components sidebar uses a slightly different interaction style (hover:bg-card-accent vs hover:bg-accent). If both are intended to match the “new admin UI,” consider harmonizing class tokens between generator templates and the UI package to ensure generated code looks identical to shipped components.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@baseplate-dev/core-generators': patch | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm correct release type and package coverage.
This looks like a feature that extends public API surface (template replacements). Consider bumping minor instead of patch and verify whether other affected packages (e.g., ui-components, project-builder-lib) also need changesets.
If you decide it’s a minor change, here’s the edit:
-'@baseplate-dev/core-generators': patch
+'@baseplate-dev/core-generators': minorTo verify coverage, run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify default branch
DEFAULT_BRANCH_REF="$(git rev-parse --abbrev-ref origin/HEAD)"
echo "Default branch: $DEFAULT_BRANCH_REF"
# List changed packages
echo "Changed packages:"
git diff --name-only "$DEFAULT_BRANCH_REF"...HEAD \
| gawk -F/ '/^packages\/[^/]+\//{print $1"/"$2}' \
| sort -u | tee /tmp/changed_packages.txt
# Map packages to their "name" field
echo
echo "Changed package names:"
while read -r pkgdir; do
if [[ -f "$pkgdir/package.json" ]]; then
name=$(jq -r '.name' "$pkgdir/package.json")
echo "$name ($pkgdir)"
fi
done < /tmp/changed_packages.txt | tee /tmp/changed_package_names.txt
# List packages mentioned in changesets
echo
echo "Packages mentioned in changesets:"
rg -n --no-heading "['\"]@baseplate-dev/[^'\"]+['\"]" .changeset/*.md 2>/dev/null \
| sed -E "s/.*(['\"])(@baseplate-dev\/[^'\"]+)\1.*/\2/" \
| sort -u | tee /tmp/changeset_packages.txt
echo
echo "Packages changed but missing a changeset:"
comm -23 \
<(cut -d' ' -f1 /tmp/changed_package_names.txt | sort -u) \
<(cat /tmp/changeset_packages.txt | sort -u) || trueLength of output: 432
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Identify default branch
DEFAULT_BRANCH_REF=$(git rev-parse --abbrev-ref origin/HEAD)
echo "Default branch: $DEFAULT_BRANCH_REF"
# Get changed package directories
echo
echo "Changed packages:"
changed_pkgs=$(git diff --name-only "$DEFAULT_BRANCH_REF"...HEAD \
| awk -F/ '/^packages\/[^/]+\//{print $1"/"$2}' \
| sort -u)
printf "%s\n" $changed_pkgs
# Map directories to package names
echo
echo "Changed package names:"
declare -a changed_names
for pkg in $changed_pkgs; do
if [[ -f "$pkg/package.json" ]]; then
name=$(jq -r '.name' "$pkg/package.json")
changed_names+=("$name")
echo "$name ($pkg)"
fi
done
# List packages mentioned in changesets
echo
echo "Packages in changesets:"
changeset_pkgs=$(rg -h "['\"]@baseplate-dev/[^'\"]+['\"]" .changeset/*.md 2>/dev/null \
| sed -E "s/.*(['\"])(@baseplate-dev\/[^'\"]+)\1.*/\2/" \
| sort -u)
printf "%s\n" $changeset_pkgs
# Identify any changed packages missing a changeset entry
echo
echo "Packages changed but missing a changeset:"
printf "%s\n" "${changed_names[@]}" | sort -u > /tmp/changed_names.txt
printf "%s\n" $changeset_pkgs | sort -u > /tmp/changeset_pkgs.txt
comm -23 /tmp/changed_names.txt /tmp/changeset_pkgs.txt || trueLength of output: 32279
Bump core-generators to minor and include all affected packages
Your changes touch these package directories without corresponding entries in the changeset:
- @baseplate-dev/project-builder-lib
- @baseplate-dev/project-builder-server
- @baseplate-dev/project-builder-web
- @baseplate-dev/react-generators
- @baseplate-dev/ui-components
Because you’re extending the public API of core-generators, you should bump it to a minor release—and you’ll also need to add changeset entries for each of the other changed packages. For example:
.changeset/template-replacements-feature.md
-'@baseplate-dev/core-generators': patch
+'@baseplate-dev/core-generators': minor
+'@baseplate-dev/project-builder-lib': patch
+'@baseplate-dev/project-builder-server': patch
+'@baseplate-dev/project-builder-web': patch
+'@baseplate-dev/react-generators': patch
+'@baseplate-dev/ui-components': patchAdjust the release type for each package as needed based on whether the change is purely internal (patch) or extends its public API (minor).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| '@baseplate-dev/core-generators': patch | |
| '@baseplate-dev/core-generators': minor | |
| '@baseplate-dev/project-builder-lib': patch | |
| '@baseplate-dev/project-builder-server': patch | |
| '@baseplate-dev/project-builder-web': patch | |
| '@baseplate-dev/react-generators': patch | |
| '@baseplate-dev/ui-components': patch |
| // Create a temporary ts-morph project to parse the content | ||
| const project = new Project({ | ||
| compilerOptions: { | ||
| allowJs: true, | ||
| jsx: 1, // JsxEmit.React | ||
| }, | ||
| useInMemoryFileSystem: true, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Parse JSX reliably: use .tsx and correct jsx setting
Parsing JSX content with ts-morph requires a .tsx extension. Also, 1 corresponds to JsxEmit.Preserve (not React). Use .tsx and the correct enum value to avoid failed parsing on TSX/JSX-heavy templates.
Apply this diff:
const project = new Project({
compilerOptions: {
allowJs: true,
- jsx: 1, // JsxEmit.React
+ jsx: 2, // JsxEmit.React
},
useInMemoryFileSystem: true,
});
- const sourceFile = project.createSourceFile('temp.ts', content, {
+ const sourceFile = project.createSourceFile('temp.tsx', content, {
overwrite: true,
});Also applies to: 41-43
🤖 Prompt for AI Agents
In
packages/core-generators/src/renderers/typescript/extractor/apply-simple-replacements.ts
around lines 32-40 (and also 41-43), update the ts-morph Project and source-file
creation to reliably parse JSX: set compilerOptions.jsx to the proper enum (use
ts.JsxEmit.React or import ts and use ts.JsxEmit.React instead of the numeric 1)
and ensure any created source file uses a .tsx extension when the content may
contain JSX (e.g., createSourceFile('file.tsx', content, ...)); this fixes
incorrect jsx enum usage and ensures TSX is parsed correctly.
| if (isStringLiteral(value)) { | ||
| // For string literals (like paths), match them within quotes | ||
| // This handles both single and double quotes, and template literals | ||
| const patterns = [ | ||
| new RegExp(`'${escapedValue}'`, 'g'), | ||
| new RegExp(`"${escapedValue}"`, 'g'), | ||
| new RegExp(`\`${escapedValue}\``, 'g'), | ||
| ]; | ||
|
|
||
| let result = text; | ||
| for (const pattern of patterns) { | ||
| result = result.replace(pattern, (match) => { | ||
| const quote = match[0]; | ||
| return `${quote}${variable}${quote}`; | ||
| }); | ||
| } | ||
| return result; | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate dynamic RegExp for string-literal replacements (avoid ReDoS warnings)
The three dynamic regexes flagged by static analysis can be replaced by safe, fast string replaceAll calls. This removes ReDoS risk while preserving behavior.
Apply this diff:
- if (isStringLiteral(value)) {
- // For string literals (like paths), match them within quotes
- // This handles both single and double quotes, and template literals
- const patterns = [
- new RegExp(`'${escapedValue}'`, 'g'),
- new RegExp(`"${escapedValue}"`, 'g'),
- new RegExp(`\`${escapedValue}\``, 'g'),
- ];
-
- let result = text;
- for (const pattern of patterns) {
- result = result.replace(pattern, (match) => {
- const quote = match[0];
- return `${quote}${variable}${quote}`;
- });
- }
- return result;
- } else {
+ if (isStringLiteral(value)) {
+ // For string literals (like paths), replace quoted occurrences
+ // Preserve the original quote type
+ let result = text;
+ result = result.replaceAll(`'${value}'`, `'${variable}'`);
+ result = result.replaceAll(`"${value}"`, `"${variable}"`);
+ result = result.replaceAll(`\`${value}\``, `\`${variable}\``);
+ return result;
+ } else {
// For other values, do exact matching with word boundaries where possible
const regex = new RegExp(`\\b${escapedValue}\\b`, 'g');
return text.replace(regex, variable);
}Note: Keeping the identifier branch regex is reasonable since values are restricted (escaped and identifier-like), but we can further reduce risk later by tokenizing on identifiers if needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isStringLiteral(value)) { | |
| // For string literals (like paths), match them within quotes | |
| // This handles both single and double quotes, and template literals | |
| const patterns = [ | |
| new RegExp(`'${escapedValue}'`, 'g'), | |
| new RegExp(`"${escapedValue}"`, 'g'), | |
| new RegExp(`\`${escapedValue}\``, 'g'), | |
| ]; | |
| let result = text; | |
| for (const pattern of patterns) { | |
| result = result.replace(pattern, (match) => { | |
| const quote = match[0]; | |
| return `${quote}${variable}${quote}`; | |
| }); | |
| } | |
| return result; | |
| } else { | |
| if (isStringLiteral(value)) { | |
| // For string literals (like paths), replace quoted occurrences | |
| // Preserve the original quote type | |
| let result = text; | |
| result = result.replaceAll(`'${value}'`, `'${variable}'`); | |
| result = result.replaceAll(`"${value}"`, `"${variable}"`); | |
| result = result.replaceAll(`\`${value}\``, `\`${variable}\``); | |
| return result; | |
| } else { | |
| // For other values, do exact matching with word boundaries where possible | |
| const regex = new RegExp(`\\b${escapedValue}\\b`, 'g'); | |
| return text.replace(regex, variable); | |
| } |
🧰 Tools
🪛 ast-grep (0.38.6)
[warning] 91-91: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp('${escapedValue}', 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 92-92: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp("${escapedValue}", 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 93-93: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\${escapedValue}``, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
| // Phase 1: Parse and apply simple replacement comments | ||
| const { content: afterSimpleReplacements, replacements } = | ||
| parseSimpleReplacements(content); | ||
|
|
||
| processedContent = afterSimpleReplacements; | ||
|
|
||
| // Apply simple replacements if any were found | ||
| if (Object.keys(replacements).length > 0) { | ||
| processedContent = applySimpleReplacements(processedContent, replacements); | ||
|
|
||
| // Track variables introduced by simple replacements | ||
| for (const variable of Object.values(replacements)) { | ||
| discoveredVariables[variable] = { | ||
| type: 'replacement', | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: discoveredVariables keyed by replacement values instead of variable names
You’re iterating over Object.values(replacements), which are the replacement values, not the variable names. This stores discoveredVariables under the replacement text (wrong) instead of the canonical variable name. It also risks collisions and breaks consistency with Phase 2, which uses TPL_ keys.
Apply this diff to use the replacement names (keys), and normalize to the canonical TPL_ prefix:
- // Apply simple replacements if any were found
- if (Object.keys(replacements).length > 0) {
- processedContent = applySimpleReplacements(processedContent, replacements);
-
- // Track variables introduced by simple replacements
- for (const variable of Object.values(replacements)) {
- discoveredVariables[variable] = {
- type: 'replacement',
- };
- }
- }
+ // Apply simple replacements if any were found
+ if (Object.keys(replacements).length > 0) {
+ processedContent = applySimpleReplacements(processedContent, replacements);
+
+ // Track variables introduced by simple replacements (canonical TPL_* keys)
+ for (const key of Object.keys(replacements)) {
+ const fullName = key.startsWith('TPL_') ? key : `TPL_${key}`;
+ if (!discoveredVariables[fullName]) {
+ discoveredVariables[fullName] = { type: 'replacement' };
+ }
+ }
+ }Additionally, ensure parseSimpleReplacements returns keys as either names or canonical TPL_*; the normalization above handles both cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Phase 1: Parse and apply simple replacement comments | |
| const { content: afterSimpleReplacements, replacements } = | |
| parseSimpleReplacements(content); | |
| processedContent = afterSimpleReplacements; | |
| // Apply simple replacements if any were found | |
| if (Object.keys(replacements).length > 0) { | |
| processedContent = applySimpleReplacements(processedContent, replacements); | |
| // Track variables introduced by simple replacements | |
| for (const variable of Object.values(replacements)) { | |
| discoveredVariables[variable] = { | |
| type: 'replacement', | |
| }; | |
| } | |
| } | |
| // Phase 1: Parse and apply simple replacement comments | |
| const { content: afterSimpleReplacements, replacements } = | |
| parseSimpleReplacements(content); | |
| processedContent = afterSimpleReplacements; | |
| // Apply simple replacements if any were found | |
| if (Object.keys(replacements).length > 0) { | |
| processedContent = applySimpleReplacements(processedContent, replacements); | |
| // Track variables introduced by simple replacements (canonical TPL_* keys) | |
| for (const key of Object.keys(replacements)) { | |
| const fullName = key.startsWith('TPL_') ? key : `TPL_${key}`; | |
| if (!discoveredVariables[fullName]) { | |
| discoveredVariables[fullName] = { type: 'replacement' }; | |
| } | |
| } | |
| } |
| interface ParseSimpleReplacementsResult { | ||
| /** Content with replacement comments removed */ | ||
| content: string; | ||
| /** Map of values to TPL variable names for replacement */ | ||
| replacements: Record<string, string>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Export the public result type
parseSimpleReplacements returns ParseSimpleReplacementsResult, but the interface isn’t exported. Export it to comply with the guideline to export public-facing types.
Apply this diff:
-interface ParseSimpleReplacementsResult {
+export interface ParseSimpleReplacementsResult {
/** Content with replacement comments removed */
content: string;
/** Map of values to TPL variable names for replacement */
replacements: Record<string, string>;
}Also applies to: 25-28
🤖 Prompt for AI Agents
In
packages/core-generators/src/renderers/typescript/extractor/parse-simple-replacements.ts
around lines 11 to 16 (and also lines 25 to 28), the interface
ParseSimpleReplacementsResult is declared but not exported; update the file to
export the public-facing type by adding the export keyword to the interface
declaration (export interface ParseSimpleReplacementsResult { ... }) and apply
the same export change to the corresponding type declarations at lines 25-28 so
the function return type is publicly available.
| React.useEffect(() => { | ||
| const mql = globalThis.matchMedia( | ||
| `(max-width: ${MOBILE_BREAKPOINT - 1}px)`, | ||
| ); | ||
| const onChange = (): void => { | ||
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); | ||
| }; | ||
| mql.addEventListener('change', onChange); | ||
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); | ||
| return () => { | ||
| mql.removeEventListener('change', onChange); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Make the hook SSR/worker-safe and handle missing matchMedia; also rely on mql.matches
Unconditionally calling globalThis.matchMedia will throw in SSR/workers. Prefer window.matchMedia with guards and use mql.matches (including legacy addListener fallback).
Apply this diff:
React.useEffect(() => {
- const mql = globalThis.matchMedia(
- `(max-width: ${MOBILE_BREAKPOINT - 1}px)`,
- );
- const onChange = (): void => {
- setIsMobile(window.innerWidth < MOBILE_BREAKPOINT);
- };
- mql.addEventListener('change', onChange);
- setIsMobile(window.innerWidth < MOBILE_BREAKPOINT);
- return () => {
- mql.removeEventListener('change', onChange);
- };
+ if (typeof window === 'undefined' || !('matchMedia' in window)) {
+ // SSR or non-DOM environments
+ setIsMobile(false);
+ return;
+ }
+ const query = `(max-width: ${MOBILE_BREAKPOINT - 1}px)`;
+ const mql = window.matchMedia(query);
+ const onChange = (e: MediaQueryListEvent): void => {
+ setIsMobile(e.matches);
+ };
+ // Initialize from current match
+ setIsMobile(mql.matches);
+ if (typeof mql.addEventListener === 'function') {
+ mql.addEventListener('change', onChange);
+ return () => mql.removeEventListener('change', onChange);
+ }
+ // Legacy fallback
+ // @ts-ignore
+ mql.addListener(onChange);
+ // @ts-ignore
+ return () => mql.removeListener(onChange);
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| React.useEffect(() => { | |
| const mql = globalThis.matchMedia( | |
| `(max-width: ${MOBILE_BREAKPOINT - 1}px)`, | |
| ); | |
| const onChange = (): void => { | |
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); | |
| }; | |
| mql.addEventListener('change', onChange); | |
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); | |
| return () => { | |
| mql.removeEventListener('change', onChange); | |
| }; | |
| }, []); | |
| React.useEffect(() => { | |
| if (typeof window === 'undefined' || !('matchMedia' in window)) { | |
| // SSR or non-DOM environments | |
| setIsMobile(false); | |
| return; | |
| } | |
| const query = `(max-width: ${MOBILE_BREAKPOINT - 1}px)`; | |
| const mql = window.matchMedia(query); | |
| const onChange = (e: MediaQueryListEvent): void => { | |
| setIsMobile(e.matches); | |
| }; | |
| // Initialize from current match | |
| setIsMobile(mql.matches); | |
| if (typeof mql.addEventListener === 'function') { | |
| mql.addEventListener('change', onChange); | |
| return () => mql.removeEventListener('change', onChange); | |
| } | |
| // Legacy fallback | |
| // @ts-ignore | |
| mql.addListener(onChange); | |
| // @ts-ignore | |
| return () => mql.removeListener(onChange); | |
| }, []); |
🤖 Prompt for AI Agents
In
packages/react-generators/src/generators/core/react-components/templates/src/hooks/use-mobile.ts
around lines 17 to 29, the hook unconditionally calls globalThis.matchMedia
which will throw in SSR/workers and also ignores mql.matches and legacy
addListener/removeListener APIs; guard access with typeof window !== 'undefined'
and typeof window.matchMedia === 'function', create mql using window.matchMedia
only when available, use mql.matches for the initial setIsMobile value (falling
back to window.innerWidth < MOBILE_BREAKPOINT or false when matchMedia is
unavailable), and attach the listener using mql.addEventListener('change', ...)
when supported or mql.addListener(...) as a fallback, likewise remove the
listener with the corresponding removeEventListener/removeListener in the
cleanup.
| @@ -0,0 +1,419 @@ | |||
| import type { Meta, StoryObj } from '@storybook/react-vite'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm Storybook types import path.
Most setups import Meta/StoryObj from @storybook/react. @storybook/react-vite is atypical for types and may break TS.
Run:
If needed:
-import type { Meta, StoryObj } from '@storybook/react-vite';
+import type { Meta, StoryObj } from '@storybook/react';🏁 Script executed:
#!/bin/bash
rg -n --no-heading "@storybook/react-vite" -A 0 -B 0
rg -n --no-heading "@storybook/react[^-]" -A 0 -B 0Length of output: 8171
Import CSF Types from @storybook/react Instead of @storybook/react-vite
The @storybook/react-vite package is the Vite builder preset, not the source of CSF types. All of your stories should import Meta and StoryObj from @storybook/react, which is where the official React typings live.
Please update every story file under packages/ui-components and top-level stories:
• In each *.stories.tsx (e.g. packages/ui-components/src/components/ui/.../*.stories.tsx and packages/ui-components/src/stories/*.stories.tsx):
-import type { Meta, StoryObj } from '@storybook/react-vite';
+import type { Meta, StoryObj } from '@storybook/react';This change aligns with Storybook’s documentation and prevents potential TypeScript resolution errors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { Meta, StoryObj } from '@storybook/react-vite'; | |
| import type { Meta, StoryObj } from '@storybook/react'; |
🤖 Prompt for AI Agents
In packages/ui-components/src/components/ui/sidebar/sidebar.stories.tsx around
line 1, the file imports CSF types from '@storybook/react-vite' which is
incorrect; update the import to pull Meta and StoryObj from '@storybook/react'
instead of '@storybook/react-vite'. Also scan and replace identical imports in
every *.stories.tsx under packages/ui-components/src/components and
packages/ui-components/src/stories so all stories import CSF types from
'@storybook/react' to match Storybook docs and avoid TypeScript resolution
issues.
| // This is the internal state of the sidebar. | ||
| // We use openProp and setOpenProp for control from outside the component. | ||
| const [_open, _setOpen] = React.useState(defaultOpen); | ||
| const open = openProp ?? _open; | ||
| const setOpen = React.useCallback( | ||
| (value: boolean | ((value: boolean) => boolean)) => { | ||
| const openState = typeof value === 'function' ? value(open) : value; | ||
| if (setOpenProp) { | ||
| setOpenProp(openState); | ||
| } else { | ||
| _setOpen(openState); | ||
| } | ||
|
|
||
| // This sets the cookie to keep the sidebar state. | ||
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | ||
| }, | ||
| [setOpenProp, open], | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialize open state from cookie to honor persisted preference
You persist the sidebar state to a cookie but never read it. This means the persisted state won’t be restored across reloads.
Add an effect to initialize from the cookie (and fall back to defaultOpen):
const [_open, _setOpen] = React.useState(defaultOpen);
+ // Initialize from cookie, falling back to defaultOpen if absent.
+ React.useEffect(() => {
+ try {
+ const match = document.cookie.match(/(?:^|;\s*)sidebar_state=(true|false)/);
+ if (match) {
+ _setOpen(match[1] === 'true');
+ }
+ } catch {
+ // no-op
+ }
+ }, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // This is the internal state of the sidebar. | |
| // We use openProp and setOpenProp for control from outside the component. | |
| const [_open, _setOpen] = React.useState(defaultOpen); | |
| const open = openProp ?? _open; | |
| const setOpen = React.useCallback( | |
| (value: boolean | ((value: boolean) => boolean)) => { | |
| const openState = typeof value === 'function' ? value(open) : value; | |
| if (setOpenProp) { | |
| setOpenProp(openState); | |
| } else { | |
| _setOpen(openState); | |
| } | |
| // This sets the cookie to keep the sidebar state. | |
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | |
| }, | |
| [setOpenProp, open], | |
| ); | |
| // This is the internal state of the sidebar. | |
| // We use openProp and setOpenProp for control from outside the component. | |
| const [_open, _setOpen] = React.useState(defaultOpen); | |
| // Initialize from cookie, falling back to defaultOpen if absent. | |
| React.useEffect(() => { | |
| try { | |
| const match = document.cookie.match(/(?:^|;\s*)sidebar_state=(true|false)/); | |
| if (match) { | |
| _setOpen(match[1] === 'true'); | |
| } | |
| } catch { | |
| // no-op | |
| } | |
| }, []); | |
| const open = openProp ?? _open; | |
| const setOpen = React.useCallback( | |
| (value: boolean | ((value: boolean) => boolean)) => { | |
| const openState = typeof value === 'function' ? value(open) : value; | |
| if (setOpenProp) { | |
| setOpenProp(openState); | |
| } else { | |
| _setOpen(openState); | |
| } | |
| // This sets the cookie to keep the sidebar state. | |
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | |
| }, | |
| [setOpenProp, open], | |
| ); |
🤖 Prompt for AI Agents
In packages/ui-components/src/components/ui/sidebar/sidebar.tsx around lines 74
to 92, you persist the sidebar open state to a cookie but never read it on
mount; add a client-only effect that reads SIDEBAR_COOKIE_NAME from
document.cookie, parses it to boolean (fallback to defaultOpen when missing or
invalid), and then initialize the component by either calling
setOpenProp(parsed) when a controlled prop setter exists or calling
_setOpen(parsed) for uncontrolled mode so the persisted preference is restored
on page load.
| function Skeleton({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<'div'>): React.ReactElement { | ||
| return ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Import React types instead of referring to the global React namespace in type positions
Directly referencing React.ComponentProps and React.ReactElement without importing react will fail under strict type-checking and isolatedModules. Import the types and use them directly.
Apply this diff to update the signature:
-function Skeleton({
- className,
- ...props
-}: React.ComponentProps<'div'>): React.ReactElement {
+function Skeleton({
+ className,
+ ...props
+}: ComponentProps<'div'>): ReactElement {Add the missing React type import near the top of the file:
import type { ComponentProps, ReactElement } from 'react';🤖 Prompt for AI Agents
In packages/ui-components/src/components/ui/skeleton/skeleton.tsx around lines 8
to 12, the component signature uses the global React namespace for types which
breaks strict TS settings; add a type-only import at the top: import type {
ComponentProps, ReactElement } from 'react'; then change the function signature
to use those imported types (e.g. function Skeleton({ className, ...props }:
ComponentProps<'div'>): ReactElement) so type references no longer rely on the
global React namespace.
| React.useEffect(() => { | ||
| const mql = globalThis.matchMedia( | ||
| `(max-width: ${MOBILE_BREAKPOINT - 1}px)`, | ||
| ); | ||
| const onChange = (): void => { | ||
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); | ||
| }; | ||
| mql.addEventListener('change', onChange); | ||
| setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); | ||
| return () => { | ||
| mql.removeEventListener('change', onChange); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden media query handling (Safari fallback, SSR-safe, use mql.matches)
- Prefer
mql.matchesoverwindow.innerWidthfor accuracy. - Add fallback to
addListener/removeListenerfor older Safari. - Guard for environments without
matchMedia, falling back to a resize listener.
Apply this diff within the effect:
- React.useEffect(() => {
- const mql = globalThis.matchMedia(
- `(max-width: ${MOBILE_BREAKPOINT - 1}px)`,
- );
- const onChange = (): void => {
- setIsMobile(window.innerWidth < MOBILE_BREAKPOINT);
- };
- mql.addEventListener('change', onChange);
- setIsMobile(window.innerWidth < MOBILE_BREAKPOINT);
- return () => {
- mql.removeEventListener('change', onChange);
- };
- }, []);
+ React.useEffect(() => {
+ const onResize = (): void => {
+ setIsMobile(window.innerWidth < MOBILE_BREAKPOINT);
+ };
+
+ if (typeof globalThis.matchMedia === 'function') {
+ const mql = globalThis.matchMedia(
+ `(max-width: ${MOBILE_BREAKPOINT - 1}px)`,
+ );
+ const onMqlChange = (): void => setIsMobile(mql.matches);
+ // Initialize from media query
+ onMqlChange();
+ // Modern browsers
+ if (typeof (mql as any).addEventListener === 'function') {
+ (mql as any).addEventListener('change', onMqlChange);
+ return () => (mql as any).removeEventListener('change', onMqlChange);
+ }
+ // Safari < 14 fallback
+ if (typeof (mql as any).addListener === 'function') {
+ (mql as any).addListener(onMqlChange);
+ return () => (mql as any).removeListener(onMqlChange);
+ }
+ // Last-resort fallback
+ window.addEventListener('resize', onResize);
+ return () => window.removeEventListener('resize', onResize);
+ }
+
+ // No matchMedia available
+ onResize();
+ window.addEventListener('resize', onResize);
+ return () => window.removeEventListener('resize', onResize);
+ }, []);🤖 Prompt for AI Agents
In packages/ui-components/src/hooks/use-mobile.ts around lines 15 to 27, the
effect should be hardened: first guard for SSR by checking typeof window !==
"undefined" and that globalThis.matchMedia exists; if matchMedia is present, use
const mql = globalThis.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`),
set initial state via setIsMobile(mql.matches) and use
mql.addEventListener('change', onChange) but fall back to
mql.addListener(onChange) for older Safari (and similarly on cleanup use
removeEventListener or removeListener), with onChange reading mql.matches; if
matchMedia is not available, attach a window 'resize' listener that sets
isMobile based on window.innerWidth < MOBILE_BREAKPOINT and clean that up on
unmount. Ensure all listeners are removed in the returned cleanup.
1 participant
In addition add simple replacement support to Typescript templates
Summary by CodeRabbit
New Features
UI Improvements
Chores