v2 #13
v2 #13
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors core BlaC architecture: new instance management, lifecycle, observer logic, event-queued Bloc, adapter/proxy layer, and testing utilities. React hook Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor React
participant Hook as useBloc (React)
participant Adapter as BlacAdapter
participant Core as Blac (core)
participant Bloc as Cubit/Bloc Instance
participant Store as useSyncExternalStore
React->>Hook: render()
Hook->>Adapter: construct/update (instanceId, staticProps, dependencies)
Adapter->>Core: getBloc(blocClass, options)
Core-->>Adapter: Instance (shared/isolated)
Adapter->>Adapter: register consumer, setup tracking
Hook->>Store: subscribe(getSnapshot, subscribe)
Note over Hook,Adapter: State/instance proxied for access tracking
Bloc-->>Adapter: notify(state change)
Adapter->>Adapter: check dependencies/paths
Adapter-->>Store: onChange()
Store-->>React: trigger re-render
sequenceDiagram
autonumber
participant UI as UI / caller
participant Bloc as Bloc.add(event)
participant Queue as Event Queue
participant Handler as Registered Handler
UI->>Bloc: add(event)
Bloc->>Queue: enqueue(event)
alt not processing
Bloc->>Bloc: _processEventQueue()
end
loop FIFO
Bloc->>Queue: dequeue()
Bloc->>Handler: invoke(event, emit)
Handler-->>Bloc: emit(newState) (0..n)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120–180 minutes Poem
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
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/blac/package.json (1)
3-3: Bump version to release candidate
Updated the package version from2.0.0to2.0.0-rc-1to mark this as a release candidate.Consider updating the changelog or release notes to reflect this RC milestone.
packages/blac-react/package.json (1)
3-3: Sync React package version to RC
Aligned the React wrapper package version with the core package by bumping to2.0.0-rc-1.Ensure peer dependency versions and release documentation are updated correspondingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
packages/blac-react/package.json(1 hunks)packages/blac/package.json(1 hunks)
- Updated package versions for @blac/core and @blac/react to 2.0.0-rc-2. - Enhanced README.md to clarify the event-handler pattern in the Bloc class and its usage. - Refined documentation across various files to reflect changes in event handling and state management. - Removed deprecated BlacObservable and BlacPlugin files to streamline the codebase. - Adjusted test cases to ensure compatibility with the new event-driven architecture.
- Bumped version for @blac/core and @blac/react to 2.0.0-rc-3. - Updated pnpm-lock.yaml to reflect new react version 19.1.0. - Removed deprecated dispatchEvent assertions in tests for improved clarity. - Adjusted test cases to ensure compatibility with the latest changes in the Blac lifecycle management.
…anagement - Added a new hook `useExternalBlocStore` to provide a React-compatible interface for subscribing to and accessing bloc state. - Updated imports in `index.ts` and `useBloc.tsx` to reference the new `useExternalBlocStore`. - Improved the structure and functionality of state management within the Blac framework.
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (42)
apps/demo/components/LifecycleDemo.tsx (2)
6-27: Good implementation of lifecycle callbacks with useBloc.You're effectively demonstrating the lifecycle callbacks with
onMountandonUnmount. A few suggestions to improve the code:
- Consider destructuring React hooks for consistency:
- const renderCountRef = React.useRef(0); - React.useEffect(() => { + const { useRef, useEffect } = React; + const renderCountRef = useRef(0); + useEffect(() => {
- Add null checks for
state.mountTimeandstate.unmountTimein the JSX (you're already doing this with the ternary, which is good practice).
21-21: Ensure data rendering is safe from rendering objects.The current implementation displays
state.datadirectly, which could cause rendering issues if the data is an object or array.- <p>Data: <span className="italic text-accent-foreground">{state.data || 'N/A'}</span></p> + <p>Data: <span className="italic text-accent-foreground">{typeof state.data === 'object' ? JSON.stringify(state.data) : state.data || 'N/A'}</span></p>apps/demo/components/UserProfileDemo.tsx (2)
60-60: Improve handling of invalid age inputs.The current implementation uses
parseInt(e.target.value, 10) || 0which will set age to 0 for any invalid input. Consider a more robust approach:- onChange={(e) => bloc.updateAge(parseInt(e.target.value, 10) || 0)} + onChange={(e) => { + const value = e.target.value; + // Only update if input is empty or a valid number + if (value === '' || /^\d+$/.test(value)) { + bloc.updateAge(value === '' ? undefined : parseInt(value, 10)); + } + }}
71-73: Consider moving derived calculations to the bloc.The age calculation
state.age + 1is done directly in the render method. It would be more consistent to move this to a getter in the UserProfileBloc.// In UserProfileBloc.ts + get nextYearAge(): number | undefined { + return this.state.age !== undefined ? this.state.age + 1 : undefined; + } // In the component - {state.age !== undefined && ( - <p className="flex justify-between items-baseline"><span className="text-lcars-beige-panel">Age next year:</span> <span className="font-bold text-lcars-orange-bright font-body normal-case text-right">{state.age + 1}</span></p> - )} + {bloc.nextYearAge !== undefined && ( + <p className="flex justify-between items-baseline"><span className="text-lcars-beige-panel">Age next year:</span> <span className="font-bold text-lcars-orange-bright font-body normal-case text-right">{bloc.nextYearAge}</span></p> + )}apps/demo/blocs/KeepAliveCounterCubit.ts (2)
1-11: Consider encapsulating instanceCounter as a private static property.The
instanceCountervariable is defined in the global scope, which could lead to namespace pollution. Consider moving it into the class as a private static property.- let instanceCounter = 0; export class KeepAliveCounterCubit extends Cubit<CounterState> { static keepAlive = true; // Key feature for this demo + private static instanceCounter = 0;Then update references in the constructor:
constructor() { - instanceCounter++; - super({ count: 0, instanceId: instanceCounter }); + KeepAliveCounterCubit.instanceCounter++; + super({ count: 0, instanceId: KeepAliveCounterCubit.instanceCounter });
10-36: Good implementation of the keepAlive feature, but consider cleaning up console logs for production.The
static keepAlive = trueflag effectively demonstrates the persistence feature. However, the implementation includes multiple console.log statements that should be removed or wrapped in a debug flag for production code.Consider adding a debug flag:
export class KeepAliveCounterCubit extends Cubit<CounterState> { static keepAlive = true; // Key feature for this demo + private static DEBUG = process.env.NODE_ENV !== 'production'; // Then update all console.log calls: constructor() { instanceCounter++; super({ count: 0, instanceId: instanceCounter }); - console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} CONSTRUCTED.`); + if (KeepAliveCounterCubit.DEBUG) { + console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} CONSTRUCTED.`); + } } // Similarly for other console.log callsapps/demo/blocs/LoggerEventCubit.ts (2)
20-20: Consider using a configuration flag for debugging logs.Instead of commenting out console logs, you might want to implement a debug flag that can be toggled.
- // console.log(`LoggerEventCubit instance created.`); // Basic log if needed + private debug = false; + if (this.debug) console.log(`LoggerEventCubit instance created.`);
38-42: Commented lifecycle hooks could be implemented or removed.The commented lifecycle hooks suggest incomplete functionality. Consider either implementing them properly or removing them if they're not needed for the demo.
- // // Lifecycle hook example (if needed and works with linter) - // protected onDispose() { - // super.onDispose(); - // console.log('LoggerEventCubit disposed'); - // } + // Lifecycle hook example + protected onDispose() { + super.onDispose(); + // Add your cleanup logic here + }apps/demo/blocs/LifecycleCubit.ts (2)
22-32: Avoid direct setTimeout usage in favor of a more testable approach.The setTimeout for simulating data fetching could make testing difficult and introduces timing dependencies.
Consider extracting the async operation to a separate method that could be mocked during testing:
setMounted = () => { console.log('LifecycleCubit: Mounted'); this.patch({ status: 'Mounted', mountTime: new Date(), }); // Simulate fetching data on mount - setTimeout(() => { - this.patch({ data: 'Data fetched on mount!' }); - }, 1000); + this.fetchData(); }; + private fetchData = async () => { + // This could be replaced with a real API call + return new Promise(resolve => { + setTimeout(() => { + this.patch({ data: 'Data fetched on mount!' }); + resolve('done'); + }, 1000); + }); + };
43-50: Use consistent state update method.The
resetmethod usesemitwhile other methods usepatch. For consistency, consider using the same method throughout.reset = () => { - this.emit({ + this.patch({ status: 'Initial', data: null, mountTime: null, unmountTime: null, }); }apps/demo/components/DependencyTrackingDemo.tsx (3)
7-13: Consistent line comments formatting.The comment formatting is slightly different than in other components. Consider standardizing the comment style.
const DisplayCounter: React.FC = React.memo(() => { - // no need to add a dependency selector, it will be determined automatically + // No need to add a dependency selector, it will be determined automatically const [state] = useBloc(ComplexStateCubit); const renderCountRef = React.useRef(0); React.useEffect(() => { renderCountRef.current += 1; }); return <p>Counter: <span className="font-bold text-primary">{state.counter}</span> (Renders: {renderCountRef.current})</p>; });
60-75: Inconsistent indentation in JSX.There's inconsistent indentation in the nested JSX elements. Some lines have extra spaces.
<div className="space-y-2"> <div className="flex flex-wrap gap-2"> - <Button onClick={cubit.incrementCounter} variant="default" size="sm">Increment Counter</Button> - <Button onClick={cubit.toggleFlag} variant="default" size="sm">Toggle Flag</Button> - <Button onClick={() => cubit.updateNestedValue(state.nested.value + 1)} variant="default" size="sm">Inc. Nested Val</Button> + <Button onClick={cubit.incrementCounter} variant="default" size="sm">Increment Counter</Button> + <Button onClick={cubit.toggleFlag} variant="default" size="sm">Toggle Flag</Button> + <Button onClick={() => cubit.updateNestedValue(state.nested.value + 1)} variant="default" size="sm">Inc. Nested Val</Button> </div> <div className="flex items-center gap-2"> - <Input - type="text" - value={state.text} - onChange={(e) => cubit.updateText(e.target.value)} - placeholder="Update text" - className="flex-grow" - /> + <Input + type="text" + value={state.text} + onChange={(e) => cubit.updateText(e.target.value)} + placeholder="Update text" + className="flex-grow" + /> <Button onClick={() => cubit.updateText('Reset Text')} variant="outline" size="sm">Reset Text</Button> </div>
47-49: Extract common state update function.Function to update the nested value could be extracted to the cubit for better reusability.
Consider adding a method to the ComplexStateCubit that encapsulates the increment operation:
// In ComplexStateCubit.ts + incrementNestedValue = () => { + this.patch({ + nested: { + ...this.state.nested, + value: this.state.nested.value + 1 + } + }); + }; // Then in this component: - <Button onClick={() => cubit.updateNestedValue(state.nested.value + 1)} variant="default" size="sm">Inc. Nested Val</Button> + <Button onClick={cubit.incrementNestedValue} variant="default" size="sm">Inc. Nested Val</Button>apps/demo/components/GetterDemo.tsx (3)
17-20: Consider extracting render count logic to a custom hook.The render count tracking logic is duplicated across multiple components in the demo app.
Extract this to a reusable hook:
+ // In a shared hooks file + export const useRenderCount = () => { + const renderCountRef = React.useRef(0); + React.useEffect(() => { + renderCountRef.current += 1; + }); + return renderCountRef.current; + }; // Then in this component: - const renderCountRef = React.useRef(0); - React.useEffect(() => { - renderCountRef.current += 1; - }); + const renderCount = useRenderCount(); // And update where it's used: - <p className="text-base text-muted-foreground">Component render count: {renderCountRef.current}</p> + <p className="text-base text-muted-foreground">Component render count: {renderCount}</p>
8-11: Extract child components to separate files for better organization.The
ShowCountcomponent could be extracted to its own file for better code organization and reusability.Consider moving this component to its own file if it might be used elsewhere in the demo app.
37-39: Consistent styling for text output.The styling for text outputs varies slightly between lines. Consider standardizing the styling.
- <p className="text-base text-foreground">Current Text: <span className="font-bold text-lcars-beige-panel font-body normal-case">{state.text}</span></p> - <p className="text-base text-foreground">Text Length (from getter): <span className="text-primary font-bold font-body normal-case">{cubit.textLength}</span></p> - <p className="text-base text-foreground">Uppercased Text (from getter): <span className="text-secondary font-bold font-body normal-case">{cubit.uppercasedText}</span></p> + <p className="text-base text-foreground">Current Text: <span className="font-bold font-body normal-case text-lcars-beige-panel">{state.text}</span></p> + <p className="text-base text-foreground">Text Length (from getter): <span className="font-bold font-body normal-case text-primary">{cubit.textLength}</span></p> + <p className="text-base text-foreground">Uppercased Text (from getter): <span className="font-bold font-body normal-case text-secondary">{cubit.uppercasedText}</span></p>apps/demo/blocs/CounterCubit.ts (1)
26-41: Consider reducing code duplicationThe IsolatedCounterCubit class duplicates all implementation details from CounterCubit with only the addition of
static isolated = true. This creates maintenance challenges if you need to update the counter logic in the future.You could consider extending CounterCubit:
export class IsolatedCounterCubit extends CounterCubit { static isolated = true; }This way, any future changes to the core counter logic would only need to be made in one place.
apps/demo/blocs/AuthCubit.ts (2)
29-38: Simplify logout state resetThe logout method contains redundant state setting. Since
initialAuthStatealready hasisAuthenticated: false, there's no need to explicitly set it again after spreading the initial state.logout = async () => { this.patch({ isLoading: true }); await new Promise(resolve => setTimeout(resolve, 200)); // Simulate API call this.patch({ - ...initialAuthState, // Reset to initial state values for isAuthenticated, userName - isAuthenticated: false, // Explicitly ensure it's false + ...initialAuthState, // Reset to initial state values isLoading: false }); // console.log('AuthCubit: User logged out.'); };
45-49: Consider removing commented-out codeThe commented-out
onDisposemethod with a note about a linter issue should either be fixed to comply with your linting rules or removed completely. Commented-out code can lead to confusion and clutter.apps/demo/blocs/UserProfileBloc.ts (1)
40-42: Add validation for age updatesThe
updateAgemethod accepts a number without any validation. Consider adding validation to ensure the age is within a reasonable range.updateAge = (age: number) => { + if (age < 0 || age > 150) { + console.warn('Age value is outside reasonable range'); + return; + } this.patch({ age }); };apps/demo/components/ui/Button.tsx (1)
32-54: Remove redundant case clauseThe 'default' case is redundant when you already have a default clause. This was flagged by static analysis and should be fixed.
switch (variant) { case 'secondary': variantStyle = { backgroundColor: COLOR_SECONDARY_ACCENT, color: COLOR_TEXT_PRIMARY, borderColor: COLOR_SECONDARY_ACCENT }; variantHoverStyle = { backgroundColor: '#D5DBDB', borderColor: '#D5DBDB' }; break; case 'outline': variantStyle = { backgroundColor: 'transparent', color: COLOR_PRIMARY_ACCENT, borderColor: COLOR_PRIMARY_ACCENT }; variantHoverStyle = { backgroundColor: `${COLOR_PRIMARY_ACCENT}1A`, color: COLOR_PRIMARY_ACCENT_HOVER, borderColor: COLOR_PRIMARY_ACCENT_HOVER }; break; case 'destructive': variantStyle = { backgroundColor: COLOR_DESTRUCTIVE, color: COLOR_TEXT_ON_DESTRUCTIVE, borderColor: COLOR_DESTRUCTIVE }; variantHoverStyle = { backgroundColor: COLOR_DESTRUCTIVE_HOVER, borderColor: COLOR_DESTRUCTIVE_HOVER }; break; case 'ghost': variantStyle = { backgroundColor: 'transparent', color: COLOR_PRIMARY_ACCENT, border: '1px solid transparent' }; variantHoverStyle = { backgroundColor: `${COLOR_SECONDARY_ACCENT}99` , color: COLOR_PRIMARY_ACCENT_HOVER }; break; - case 'default': default: variantStyle = { backgroundColor: COLOR_PRIMARY_ACCENT, color: COLOR_TEXT_ON_PRIMARY, borderColor: COLOR_PRIMARY_ACCENT }; variantHoverStyle = { backgroundColor: COLOR_PRIMARY_ACCENT_HOVER, borderColor: COLOR_PRIMARY_ACCENT_HOVER }; break; }🧰 Tools
🪛 Biome (1.9.4)
[error] 49-49: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
apps/docs/learn/blac-pattern.md (2)
81-81: Documentation formatting issue.There appears to be extra spacing after the code block that's causing a loose punctuation mark.
-- - **`Bloc<State, Event>`**: A more structured container that processes instances of event *classes* through registered *handler functions* to produce new `State`. Event instances are dispatched via `this.add(new EventType())`, and handlers are registered with `this.on(EventType, handler)`. This is ideal for more complex, type-safe state transitions. +- **`Bloc<State, Event>`**: A more structured container that processes instances of event *classes* through registered *handler functions* to produce new `State`. Event instances are dispatched via `this.add(new EventType())`, and handlers are registered with `this.on(EventType, handler)`. This is ideal for more complex, type-safe state transitions.🧰 Tools
🪛 LanguageTool
[uncategorized] ~81-~81: Loose punctuation mark.
Context: ... } ``` -Bloc<State, Event>: A more structured container that proces...(UNLIKELY_OPENING_PUNCTUATION)
173-176: Inconsistent list indentation.The indentation for these list items is 4 spaces, while markdown standards typically use 2 spaces for indentation.
- - Formal event classes and handlers feel like overkill. - - State transitions are complex and benefit from distinct, type-safe event classes and dedicated handlers. - - You want a clear, event-driven architecture for managing state changes and side effects, enhancing traceability and maintainability. + - Formal event classes and handlers feel like overkill. + - State transitions are complex and benefit from distinct, type-safe event classes and dedicated handlers. + - You want a clear, event-driven architecture for managing state changes and side effects, enhancing traceability and maintainability.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
173-173: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
175-175: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
apps/demo/components/BlocToBlocCommsDemo.tsx (1)
14-14: Remove debugging console.log.There's a console.log statement that appears to be left over from development.
- console.log('dashboardState', dashboardStatsCubit);apps/demo/blocs/DashboardStatsCubit.ts (2)
26-41: Consider handling potential race conditions in async state updates.While the error handling for AuthCubit retrieval is good, be cautious about potential race conditions when multiple async
loadDashboardcalls happen in quick succession.loadDashboard = async () => { + // Track the current request to handle race conditions + const requestId = Date.now(); + this._currentRequestId = requestId; + this.patch({ isLoading: true, statsMessage: 'Loading dashboard data...' }); await new Promise(resolve => setTimeout(resolve, 500)); // Simulate API call + // Abort if another request was started after this one + if (this._currentRequestId !== requestId) return; + let userName: string | null = 'Guest (Auth Unavailable)'; // Default in case of error let isAuthenticated = false;
42-55: Consider using dependency injection instead of direct bloc access.While the current approach works for accessing AuthCubit, consider passing the AuthCubit as a dependency in the constructor for better testability and looser coupling.
export class DashboardStatsCubit extends Cubit<DashboardStatsState> { static isolated = true; // Each instance of the demo component will have its own stats cubit + private authCubit?: AuthCubit; - constructor() { + constructor(authCubit?: AuthCubit) { super(initialStatsState); + this.authCubit = authCubit; } loadDashboard = async () => { this.patch({ isLoading: true, statsMessage: 'Loading dashboard data...' }); await new Promise(resolve => setTimeout(resolve, 500)); // Simulate API call let userName: string | null = 'Guest (Auth Unavailable)'; // Default in case of error let isAuthenticated = false; try { // Here is the Bloc-to-Bloc communication - const authCubit = Blac.getBloc(AuthCubit, { throwIfNotFound: true }); // Blac.getBloc() can throw if not found + const authCubit = this.authCubit || Blac.getBloc(AuthCubit, { throwIfNotFound: true }); isAuthenticated = authCubit.state.isAuthenticated; userName = authCubit.state.userName || (isAuthenticated ? 'Authenticated User (No Name)' : 'Guest');apps/demo/components/MultiInstanceDemo.tsx (1)
29-33: Consider adding clarification about the duplicate "gamma" instances.While this is an effective demonstration of shared state, adding an explicit comment above the duplicate instances would make the intentional sharing even clearer to readers.
<CounterInstance id="alpha" initialCount={5} /> <CounterInstance id="beta" initialCount={55} /> <CounterInstance id="gamma" /> + {/* Same ID as above - demonstrates shared state between components */} <CounterInstance id="gamma" /> <p className="text-xs text-muted-foreground mt-2">apps/demo/components/BlocWithReducerDemo.tsx (2)
7-36: Consider extracting inline styles to a separate constants object.The TodoItem component uses many inline styles. For better maintainability and readability, consider extracting these styles into a separate constants object.
+const todoItemStyles = { + container: { + display: 'flex', + alignItems: 'center', + justifyContent: 'space-between', + padding: '8px 0', + borderBottom: '1px solid #eee', + }, + text: { + cursor: 'pointer', + flexGrow: 1, + }, + removeButton: { + padding: '2px 6px', + fontSize: '0.8em' + } +}; const TodoItem: React.FC<{ todo: Todo; onToggle: (id: number) => void; onRemove: (id: number) => void; }> = ({ todo, onToggle, onRemove }) => { return ( - <div style={{ - display: 'flex', - alignItems: 'center', - justifyContent: 'space-between', - padding: '8px 0', - borderBottom: '1px solid #eee', - opacity: todo.completed ? 0.6 : 1, - }}> + <div style={{ + ...todoItemStyles.container, + opacity: todo.completed ? 0.6 : 1, + }}> <span onClick={() => onToggle(todo.id)} style={{ + ...todoItemStyles.text, - textDecoration: todo.completed ? 'line-through' : 'none', - cursor: 'pointer', - flexGrow: 1, + textDecoration: todo.completed ? 'line-through' : 'none' }} > {todo.text} </span> - <Button onClick={() => onRemove(todo.id)} variant="destructive" style={{ padding: '2px 6px', fontSize: '0.8em' }}> + <Button onClick={() => onRemove(todo.id)} variant="destructive" style={todoItemStyles.removeButton}>
76-105: Consider extracting footer component for better readability.The footer section is complex and could be extracted into a separate component to improve the readability and maintainability of the TodoBlocDemo component.
+const TodoListFooter: React.FC<{ + todos: Todo[]; + activeTodosCount: number; + currentFilter: 'all' | 'active' | 'completed'; + onFilterChange: (filter: 'all' | 'active' | 'completed') => void; + onClearCompleted: () => void; +}> = ({ todos, activeTodosCount, currentFilter, onFilterChange, onClearCompleted }) => { + if (todos.length === 0) return null; + + return ( + <div style={{ + marginTop: '16px', + paddingTop: '16px', + borderTop: '1px solid #eee', + display: 'flex', + justifyContent: 'space-between', + alignItems: 'center', + fontSize: '0.9em', + color: '#555', + }}> + <span>{activeTodosCount} item{activeTodosCount !== 1 ? 's' : ''} left</span> + <div style={{ display: 'flex', gap: '8px' }}> + {(['all', 'active', 'completed'] as const).map((filter) => ( + <Button + key={filter} + onClick={() => onFilterChange(filter)} + variant={currentFilter === filter ? 'default' : 'outline'} + > + {filter.charAt(0).toUpperCase() + filter.slice(1)} + </Button> + ))} + </div> + {todos.some(todo => todo.completed) && ( + <Button onClick={onClearCompleted} variant="ghost"> + Clear Completed + </Button> + )} + </div> + ); +}; // In TodoBlocDemo: - {state.todos.length > 0 && ( - <div style={{ - marginTop: '16px', - paddingTop: '16px', - borderTop: '1px solid #eee', - display: 'flex', - justifyContent: 'space-between', - alignItems: 'center', - fontSize: '0.9em', - color: '#555', - }}> - <span>{activeTodosCount} item{activeTodosCount !== 1 ? 's' : ''} left</span> - <div style={{ display: 'flex', gap: '8px' }}> - {(['all', 'active', 'completed'] as const).map((filter) => ( - <Button - key={filter} - onClick={() => bloc.setFilter(filter)} - variant={state.filter === filter ? 'default' : 'outline'} - > - {filter.charAt(0).toUpperCase() + filter.slice(1)} - </Button> - ))} - </div> - {state.todos.some(todo => todo.completed) && ( - <Button onClick={bloc.clearCompleted} variant="ghost"> - Clear Completed - </Button> - )} - </div> - )} + <TodoListFooter + todos={state.todos} + activeTodosCount={activeTodosCount} + currentFilter={state.filter} + onFilterChange={bloc.setFilter} + onClearCompleted={bloc.clearCompleted} + />apps/docs/api/key-methods.md (1)
272-272: Add hyphen to "process-specific" for grammatical correctness.A minor grammatical improvement would be to hyphenate "process-specific" when used as a compound modifier.
- Use `Bloc` for more complex state logic where an event-driven approach is beneficial. `Bloc`s process specific event *classes* through registered *handlers* (using `this.on(EventType, handler)` and `this.add(new EventType())`) to produce new `State`. This pattern is excellent for managing intricate state transitions and side effects in a structured and type-safe way. + Use `Bloc` for more complex state logic where an event-driven approach is beneficial. `Bloc`s process process-specific event *classes* through registered *handlers* (using `this.on(EventType, handler)` and `this.add(new EventType())`) to produce new `State`. This pattern is excellent for managing intricate state transitions and side effects in a structured and type-safe way.🧰 Tools
🪛 LanguageTool
[uncategorized] ~272-~272: When ‘process-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...-driven approach is beneficial.Blocs process specific event classes through registered *han...(SPECIFIC_HYPHEN)
apps/demo/blocs/ComplexStateCubit.ts (1)
41-47: Consider handling out-of-bounds array access more explicitly.The
updateItemmethod silently does nothing when the index is out of bounds. Consider either throwing an error or logging a warning to make this behavior more explicit.updateItem = (index: number, item: string) => { const newItems = [...this.state.items]; if (index >= 0 && index < newItems.length) { newItems[index] = item; this.patch({ items: newItems }); + } else { + console.warn(`Cannot update item at index ${index}: out of bounds`); } };apps/demo/App.tsx (1)
262-265: Consider environment-based logging configuration.Enabling Blac logging globally and exposing the instance on the window object is good for the demo application. For production code, consider making this conditional based on environment.
-Blac.enableLog = true; -window.blac = Blac; -console.log(Blac.instance); +// Enable logging and debugging in development only +if (process.env.NODE_ENV !== 'production') { + Blac.enableLog = true; + window.blac = Blac; + console.log(Blac.instance); +}apps/docs/learn/core-concepts.md (1)
67-73: Fix formatting in the list items.There are formatting inconsistencies in the bullet points. The punctuation marks should be properly aligned.
- `on(EventClass, handler)`: Registers a handler function for a specific event class. The handler receives the event instance and an `emit` function to produce new state. - `add(eventInstance: Event)`: Dispatches an instance of an event class. The `Bloc` looks up the registered handler based on the event instance's constructor. + `on(EventClass, handler)`: Registers a handler function for a specific event class. The handler receives the event instance and an `emit` function to produce new state. + `add(eventInstance: Event)`: Dispatches an instance of an event class. The `Bloc` looks up the registered handler based on the event instance's constructor.🧰 Tools
🪛 LanguageTool
[uncategorized] ~71-~71: Loose punctuation mark.
Context: ...ss logic. -on(EventClass, handler): Registers a handler function for a spec...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~71-~71: You might be missing the article “a” here.
Context: ...tance and anemitfunction to produce new state. -add(eventInstance: Event):...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~72-~72: Loose punctuation mark.
Context: ...w state. -add(eventInstance: Event): Dispatches an instance of an event clas...(UNLIKELY_OPENING_PUNCTUATION)
apps/demo/components/CustomSelectorDemo.tsx (2)
19-25: Avoid recomputing derived values twice per render.
counterIsEvenandfirstCharare calculated inside the selector and then recalculated here.
Consider memoising them or deriving from the selector result to keep a single source of truth and prevent accidental drift if the logic ever changes.
75-77: Update documentation string to match the newselectorprop name.The explanatory text still references
dependencySelector, which was renamed toselectorin the API.apps/demo/components/ConditionalDependencyDemo.tsx (2)
68-78: Inline style objects are recreated every render; consider memoisation or CSS classes.Creating large literal objects inside JSX (especially with comments) allocates a new object each render, preventing React memoisation techniques and potentially hitting performance in bigger lists.
If you plan to keep inline styles, wrap them inuseMemoor extract them to a constant or CSS/ Tailwind class.
110-136: Missingaria-label/ accessible text for generic buttons.Some buttons only contain text (“Set First to Alex”) but others rely on contextual information (“Toggle Full Name Display …”).
For screen-reader friendliness, ensure eachButtonconveys intent via anaria-labelor clear inner text that does not depend on reading preceding content.packages/blac-react/src/useBloc.tsx (2)
35-41: Out-of-date JSDoc block references a “Default dependency selector” that no longer exists.The comment describes a helper that has been removed in this refactor. Please delete or update the block to avoid confusion for maintainers and users.
118-131: Effect dependency array may still fire unnecessarily.Passing
instance.currentdirectly can change identity if a different instance is mounted later, but more importantly refs are mutable and their.currentisn’t stable for deps.
If the goal is “run once on mount/unmount”, depend only onrid(which is constant) or use an empty array and silence linter warnings explicitly.-}, [instance.current, rid]); // Do not add options.onMount to deps, it will cause a loop +// eslint-disable-next-line react-hooks/exhaustive-deps +}, [rid]);apps/demo/blocs/TodoBloc.ts (1)
117-124: Remove redundant switch-case branch to satisfy linter & avoid dead codeThe
case 'all'branch returns the exact same expression as thedefaultclause, which makes it a “useless case” flagged by Biome (lint/complexity/noUselessSwitchCase).
Keeping it will not harm correctness, but trimming it prevents needless duplication and keeps the exhaustiveness of the switch obvious.- case 'all': - default: - return this.state.todos; + default: + return this.state.todos;🧰 Tools
🪛 Biome (1.9.4)
[error] 122-122: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
packages/blac/src/Blac.ts (2)
153-154: Log typo – “disposed”, not “dispatched”The context is
disposeBloc, but the message reads “dispatched bloc”, which is misleading when scanning logs.- this.log('dispatched bloc', bloc) + this.log('disposed bloc', bloc)
282-299:activateBlocduplicates registry lookup — consider early-exit or reuse
activateBlocruns a fresh lookup (findIsolatedBlocInstance/findRegisteredBlocInstance) even though the caller often already knows the bloc is not registered (it’s usually called right after construction). This is harmless but adds O(n) work for large isolated sets.Optional micro-refactor: accept a
skipLookupboolean or simply rely on the caller to guard when appropriate.Not blocking, just pointing out a possible simplification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (73)
apps/demo/App.tsx(1 hunks)apps/demo/LcarsHeader.tsx(1 hunks)apps/demo/blocs/.gitkeep(1 hunks)apps/demo/blocs/AuthCubit.ts(1 hunks)apps/demo/blocs/ComplexStateCubit.ts(1 hunks)apps/demo/blocs/ConditionalUserProfileCubit.ts(1 hunks)apps/demo/blocs/CounterCubit.ts(1 hunks)apps/demo/blocs/DashboardStatsCubit.ts(1 hunks)apps/demo/blocs/KeepAliveCounterCubit.ts(1 hunks)apps/demo/blocs/LifecycleCubit.ts(1 hunks)apps/demo/blocs/LoggerEventCubit.ts(1 hunks)apps/demo/blocs/PersistentSettingsCubit.ts(1 hunks)apps/demo/blocs/TodoBloc.ts(1 hunks)apps/demo/blocs/UserProfileBloc.ts(1 hunks)apps/demo/components/.gitkeep(1 hunks)apps/demo/components/BasicCounterDemo.tsx(1 hunks)apps/demo/components/BlocToBlocCommsDemo.tsx(1 hunks)apps/demo/components/BlocWithReducerDemo.tsx(1 hunks)apps/demo/components/ConditionalDependencyDemo.tsx(1 hunks)apps/demo/components/CustomSelectorDemo.tsx(1 hunks)apps/demo/components/DependencyTrackingDemo.tsx(1 hunks)apps/demo/components/GetterDemo.tsx(1 hunks)apps/demo/components/IsolatedCounterDemo.tsx(1 hunks)apps/demo/components/KeepAliveDemo.tsx(1 hunks)apps/demo/components/LifecycleDemo.tsx(1 hunks)apps/demo/components/MultiInstanceDemo.tsx(1 hunks)apps/demo/components/TodoBlocDemo.tsx(1 hunks)apps/demo/components/UserProfileDemo.tsx(1 hunks)apps/demo/components/ui/Button.tsx(1 hunks)apps/demo/components/ui/Card.tsx(1 hunks)apps/demo/components/ui/Input.tsx(1 hunks)apps/demo/components/ui/Label.tsx(1 hunks)apps/demo/index.html(1 hunks)apps/demo/lib/styles.ts(1 hunks)apps/demo/main.tsx(1 hunks)apps/demo/package.json(1 hunks)apps/demo/tsconfig.json(1 hunks)apps/demo/vite.config.ts(1 hunks)apps/docs/api/core-classes.md(3 hunks)apps/docs/api/key-methods.md(3 hunks)apps/docs/api/react-hooks.md(1 hunks)apps/docs/docs/api/core-classes.md(3 hunks)apps/docs/learn/blac-pattern.md(4 hunks)apps/docs/learn/core-concepts.md(2 hunks)apps/docs/learn/getting-started.md(1 hunks)apps/perf/main.tsx(1 hunks)apps/perf/package.json(1 hunks)packages/blac-react/package.json(2 hunks)packages/blac-react/src/externalBlocStore.ts(2 hunks)packages/blac-react/src/index.ts(1 hunks)packages/blac-react/src/useBloc.tsx(4 hunks)packages/blac-react/tests/useBlocDependencyDetection.test.tsx(2 hunks)packages/blac-react/tests/useBlocLifeCycle.test.tsx(0 hunks)packages/blac-react/tests/useBlocPerformance.test.tsx(2 hunks)packages/blac-react/tsconfig.json(1 hunks)packages/blac/README.md(3 hunks)packages/blac/package.json(1 hunks)packages/blac/src/Blac.ts(6 hunks)packages/blac/src/BlacObservable.ts(0 hunks)packages/blac/src/BlacObserver.ts(5 hunks)packages/blac/src/BlacPlugin.ts(0 hunks)packages/blac/src/BlocBase.ts(6 hunks)packages/blac/src/addons/BlacAddon.ts(0 hunks)packages/blac/src/addons/Persist.ts(0 hunks)packages/blac/src/addons/index.ts(0 hunks)packages/blac/src/index.ts(1 hunks)packages/blac/src/types.ts(1 hunks)packages/blac/tests/Blac.test.ts(1 hunks)packages/blac/tests/BlacObserver.test.ts(4 hunks)packages/blac/tests/BlocBase.test.ts(2 hunks)packages/blac/tests/Lifecycle.test.ts(0 hunks)packages/blac/tests/blac.memory.test.ts(0 hunks)readme.md(1 hunks)
💤 Files with no reviewable changes (8)
- packages/blac/src/addons/index.ts
- packages/blac/src/BlacPlugin.ts
- packages/blac/src/addons/BlacAddon.ts
- packages/blac/tests/blac.memory.test.ts
- packages/blac/src/BlacObservable.ts
- packages/blac/tests/Lifecycle.test.ts
- packages/blac/src/addons/Persist.ts
- packages/blac-react/tests/useBlocLifeCycle.test.tsx
✅ Files skipped from review due to trivial changes (16)
- apps/docs/learn/getting-started.md
- apps/demo/blocs/.gitkeep
- apps/demo/components/.gitkeep
- packages/blac/src/types.ts
- apps/demo/vite.config.ts
- apps/demo/main.tsx
- packages/blac/src/index.ts
- apps/demo/index.html
- packages/blac-react/src/index.ts
- apps/demo/package.json
- apps/demo/components/ui/Label.tsx
- apps/demo/components/ui/Input.tsx
- packages/blac/tests/Blac.test.ts
- apps/demo/tsconfig.json
- apps/demo/components/ui/Card.tsx
- apps/demo/lib/styles.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/blac/package.json
- packages/blac-react/package.json
🧰 Additional context used
🧬 Code Graph Analysis (8)
apps/demo/components/TodoBlocDemo.tsx (6)
apps/demo/blocs/TodoBloc.ts (2)
Todo(3-7)TodoBloc(48-127)apps/demo/components/ui/Button.tsx (1)
Button(88-88)packages/blac/src/BlocBase.ts (1)
state(136-138)packages/blac-react/src/useBloc.tsx (1)
useBloc(66-134)packages/blac-react/src/index.ts (1)
useBloc(4-4)apps/demo/components/ui/Input.tsx (1)
Input(45-45)
apps/demo/components/ui/Button.tsx (1)
apps/demo/lib/styles.ts (11)
BUTTON_BASE_STYLE(49-62)COLOR_SECONDARY_ACCENT(11-11)COLOR_TEXT_PRIMARY(7-7)COLOR_PRIMARY_ACCENT(9-9)COLOR_PRIMARY_ACCENT_HOVER(10-10)COLOR_DESTRUCTIVE(12-12)COLOR_TEXT_ON_DESTRUCTIVE(16-16)COLOR_DESTRUCTIVE_HOVER(13-13)COLOR_TEXT_ON_PRIMARY(15-15)BUTTON_DISABLED_STYLE(69-74)BUTTON_FOCUS_STYLE(64-67)
packages/blac/src/BlacObserver.ts (1)
packages/blac/src/Blac.ts (1)
Blac(38-416)
packages/blac/tests/BlacObserver.test.ts (1)
packages/blac/src/BlacObserver.ts (1)
BlacObservable(26-135)
apps/demo/components/ConditionalDependencyDemo.tsx (6)
packages/blac-react/src/useBloc.tsx (1)
useBloc(66-134)apps/demo/blocs/ConditionalUserProfileCubit.ts (1)
ConditionalUserProfileCubit(11-56)apps/demo/lib/styles.ts (2)
DEMO_COMPONENT_CONTAINER_STYLE(144-150)LCARS_ORANGE(159-159)apps/demo/components/ui/Label.tsx (1)
Label(20-20)apps/demo/components/ui/Input.tsx (1)
Input(45-45)apps/demo/components/ui/Button.tsx (1)
Button(88-88)
apps/demo/components/CustomSelectorDemo.tsx (5)
packages/blac-react/src/useBloc.tsx (1)
useBloc(66-134)apps/demo/blocs/ComplexStateCubit.ts (2)
ComplexStateCubit(15-68)ComplexDemoState(3-13)packages/blac/src/BlocBase.ts (1)
state(136-138)apps/demo/components/ui/Button.tsx (1)
Button(88-88)apps/demo/components/ui/Input.tsx (1)
Input(45-45)
packages/blac-react/src/externalBlocStore.ts (6)
packages/blac/src/types.ts (4)
BlocConstructor(23-23)BlocState(36-36)BlocBaseAbstract(15-17)BlocHookDependencyArrayFn(60-62)packages/blac/src/BlocBase.ts (2)
state(136-138)isIsolated(33-35)packages/blac-react/src/index.ts (1)
useExternalBlocStore(4-4)packages/blac-react/src/useBloc.tsx (1)
BlocHookOptions(27-33)packages/blac/src/Blac.ts (1)
Blac(38-416)packages/blac/src/BlacObserver.ts (1)
BlacObserver(9-20)
packages/blac/src/Blac.ts (2)
packages/blac/src/types.ts (4)
BlocHookDependencyArrayFn(60-62)BlocState(36-36)InferPropsFromGeneric(42-47)BlocConstructor(23-23)packages/blac/src/BlocBase.ts (1)
isIsolated(33-35)
🪛 LanguageTool
apps/docs/learn/blac-pattern.md
[uncategorized] ~81-~81: Loose punctuation mark.
Context: ... } ``` - Bloc<State, Event>: A more structured container that proces...
(UNLIKELY_OPENING_PUNCTUATION)
apps/docs/api/key-methods.md
[uncategorized] ~272-~272: When ‘process-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...-driven approach is beneficial. Blocs process specific event classes through registered *han...
(SPECIFIC_HYPHEN)
apps/docs/learn/core-concepts.md
[uncategorized] ~71-~71: Loose punctuation mark.
Context: ...ss logic. - on(EventClass, handler): Registers a handler function for a spec...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~71-~71: You might be missing the article “a” here.
Context: ...tance and an emit function to produce new state. - add(eventInstance: Event):...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~72-~72: Loose punctuation mark.
Context: ...w state. - add(eventInstance: Event): Dispatches an instance of an event clas...
(UNLIKELY_OPENING_PUNCTUATION)
readme.md
[uncategorized] ~103-~103: Loose punctuation mark.
Context: ... structure. - Bloc<State, Event>: A more advanced state container that us...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
apps/docs/learn/blac-pattern.md
173-173: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
175-175: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🪛 Biome (1.9.4)
apps/demo/components/ui/Button.tsx
[error] 49-49: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
apps/demo/blocs/TodoBloc.ts
[error] 122-122: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (105)
packages/blac-react/tests/useBlocPerformance.test.tsx (2)
3-3: Clean up imports to only include what's neededImport is now correctly scoped to only the specific React elements being used (
FCanduseCallback), following best practices.
91-93: Renamed option fromdependencySelectortoselectorThe option name has been standardized to
selectorto maintain consistency with the updated API in theuseBlochook.packages/blac-react/tests/useBlocDependencyDetection.test.tsx (2)
6-6: Clean up imports to only include what's neededImport is now correctly scoped to only the specific React elements being used (
FCanduseState), following best practices.
399-404: Renamed option fromdependencySelectortoselectorThe option name has been standardized to
selectorin the custom dependency selector test, maintaining consistency with the updated API in theuseBlochook.apps/perf/main.tsx (1)
134-135: Renamed option fromdependencySelectortoselectorThe option name has been standardized to
selectorwithin theRowListcomponent, maintaining consistency with the updated API in theuseBlochook. The functionality remains the same as it still correctly receives the state array and returns a nested array containing the length for dependency tracking.packages/blac-react/tsconfig.json (2)
13-14: Enabled stricter type checkingSetting
skipLibChecktofalseenables type checking of all declaration files, which helps catch potential type issues earlier in the development process.
16-16: Simplified compiler optionsThe
typesarray specifying"vitest/globals"and"@testing-library/jest-dom"has been removed, leaving onlyresolveJsonModuleenabled. This change streamlines the configuration while maintaining essential functionality.apps/perf/package.json (1)
8-8: Simplifying script configuration - looks good!The change to consolidate multiple scripts into a single
"dev": "vite"script follows good practices for simplifying development workflows and creates consistency with other apps in the monorepo.readme.md (1)
103-103: Great addition explaining the event-handler pattern!This documentation update effectively clarifies the new event-driven approach for
Bloc<State, Event>, which complements the existing reducer-based approach. The explanation of how event instances are processed usingthis.add()and handlers are registered withthis.on()provides developers with a clear understanding of this pattern.🧰 Tools
🪛 LanguageTool
[uncategorized] ~103-~103: Loose punctuation mark.
Context: ... structure. -Bloc<State, Event>: A more advanced state container that us...(UNLIKELY_OPENING_PUNCTUATION)
apps/demo/components/BasicCounterDemo.tsx (1)
1-21: Well-implemented shared counter demo componentThis component serves as a good example of using a shared/global instance of a Cubit. The code is clean, follows React best practices, and includes helpful comments explaining the shared nature of the state management.
apps/demo/components/IsolatedCounterDemo.tsx (1)
1-34: Excellent implementation of isolated state managementThis component effectively demonstrates how to use isolated Cubit instances with the
static isolated = trueproperty. The detailed comments provide valuable insights into how isolation works, and the component properly handles props with sensible defaults.apps/demo/components/LifecycleDemo.tsx (1)
29-45: Well-structured component with clear explanations.The
LifecycleDemocomponent effectively demonstrates the concept of component lifecycle with Blac. The explanatory text at the bottom provides good context.apps/demo/components/UserProfileDemo.tsx (1)
13-77: Well-structured component with good use of the bloc pattern.The component effectively demonstrates the use of the bloc pattern with React. The form is well-organized, and the display of derived state values showcases the power of getters in the bloc.
packages/blac/tests/BlocBase.test.ts (1)
2-2: Import statement updated correctly to match the refactored code structure.The import statement now properly imports
Blac,BlacObserver, andBlocBasedirectly from the source.apps/demo/blocs/LoggerEventCubit.ts (1)
1-43: Well-structured demo cubit for event subscription demonstration.This cubit provides a clean implementation for demonstrating event subscriptions with a simple state model. The code is organized logically with clear separation between state definition and methods.
apps/demo/blocs/LifecycleCubit.ts (1)
1-51: Well-implemented lifecycle demonstration cubit.This cubit effectively demonstrates lifecycle management with proper state structure and isolated instance management.
apps/demo/components/DependencyTrackingDemo.tsx (1)
1-86: Well-structured demo for dependency tracking.The component effectively demonstrates fine-grained state dependency tracking with React.memo and useBloc.
apps/demo/components/GetterDemo.tsx (1)
1-58: Well-implemented getter demonstration component.The component effectively demonstrates the use of cubit getters with selective re-rendering.
apps/demo/LcarsHeader.tsx (5)
1-35: Well-implemented React component with clean props interfaceThe
LcarsHeaderButtonBlockcomponent has a well-defined TypeScript interface with appropriate prop types. The component structure follows React best practices by using function components with TypeScript interfaces.
27-34: Proper cleanup of interval timerGood implementation of the
useEffecthook with proper cleanup by clearing the interval when the component unmounts. This prevents memory leaks that could occur if the interval continued running after component unmount.
36-57: Clean layout structure with semantic HTMLThe header component uses semantic HTML with proper structure for the top bar section. The layout with left-group, date display, and right-group provides a clear organization of the header elements.
59-66: Consistent styling approach for title barThe main title bar has a clean, consistent approach to styling with appropriate spacing and positioning elements.
68-76: Good implementation of time display with fallback valuesThe time display implementation properly handles initial rendering with fallback values (
12:31:42) while waiting for the actual time to be set by the interval.apps/demo/components/TodoBlocDemo.tsx (5)
7-36: Well-designed TodoItem component with proper event handlingThe
TodoItemcomponent has a clean interface with appropriate props for toggling and removing todos. The styling approach using inline styles is consistent and provides good visual feedback with the strikethrough for completed items and opacity changes.
38-48: Effective use of Bloc pattern with proper form handlingThe component correctly uses the
useBlochook to connect to theTodoBloc. The form submission handler includes proper validation to prevent adding empty todos and clears the input field after submission.
66-73: Efficient rendering of todo itemsThe component efficiently maps over the filtered todos and passes appropriate callbacks to the
TodoItemcomponent. The use ofbloc.filteredTodosgetter method demonstrates good separation of filtering logic in the bloc class.
76-105: Comprehensive UI for todo managementThe footer section provides a complete set of controls: item count, filter buttons, and clear completed button. The conditional rendering based on the presence of completed todos shows attention to detail in the UI implementation.
106-111: Excellent documentation of the Bloc pattern usageThe explanatory paragraph at the bottom clearly describes how the component uses the event-handler pattern with the Bloc. This documentation helps developers understand the pattern implementation and how actions are dispatched to the Bloc.
apps/demo/blocs/PersistentSettingsCubit.ts (1)
53-56: 🛠️ Refactor suggestionAdd manual persistence for reset function
Add direct persistence after resetting to defaults.
resetToDefaults = () => { this.emit(initialSettings); // This will also be persisted + this._persistState(); console.log('Settings reset to defaults and persisted.'); }Likely an incorrect or invalid review comment.
packages/blac/src/BlacObserver.ts (6)
38-38: Good optimization using Set for observer collectionChanging from an array to a Set for the observers collection is a good optimization. Sets automatically handle uniqueness and provide O(1) complexity for add/delete operations.
45-46: Updated size getter to use Set's size propertyThe size getter correctly uses the Set's size property rather than length, aligning with the collection type change.
62-64: Added logging and removed lifecycle event dispatchingAdded helpful logging for subscription events and commented out the lifecycle event dispatching, aligning with the architecture changes mentioned in the summary where the plugin system was removed.
71-73: Improved unsubscription with loggingAdded logging for unsubscription events to aid in debugging, which is a good practice.
81-87: Automatic bloc disposal when no observers remainAdded important cleanup logic to automatically dispose the bloc when no observers remain. This prevents memory leaks by ensuring unused blocs are properly disposed.
129-134: Renamed method for clarity and updated implementationRenamed
dispose()toclear()for clarity and updated to use the Set'sclear()method after unsubscribing all observers, which is more efficient than iterating through and removing them one by one.apps/demo/blocs/CounterCubit.ts (2)
7-10: Well-designed props interfaceYour interface effectively defines the optional initialCount with clear typing and includes a helpful comment about the id parameter's purpose. This level of documentation is good practice.
12-24: Implementation looks goodThe implementation of CounterCubit is clean and follows good practices. Using arrow functions for increment and decrement methods ensures proper
thisbinding which is important for methods that might be passed directly to event handlers.apps/demo/blocs/AuthCubit.ts (2)
16-19: Appropriate constructor implementationThe constructor correctly initializes the cubit with the defined initial state values. The commented console log is fine to leave for potential debugging but consider removing before production.
22-27: Well-implemented async login logicThe login method properly handles the loading state during the simulated API call and updates the auth state appropriately after completion.
apps/demo/blocs/UserProfileBloc.ts (3)
16-18: Good use of static isolated propertySetting
static isolated = trueensures each component instance gets its own UserProfileBloc, which is well-documented in the comment. This is a good practice for component-specific state management.
19-26: Well-structured constructor with defaultsThe constructor provides reasonable defaults while also accepting override values through props. This makes the component both flexible and usable without configuration.
49-53: Robust initials calculationYour implementation for calculating initials properly handles potential edge cases where firstName or lastName might be empty, which is good defensive programming.
apps/demo/components/ui/Button.tsx (3)
16-20: Good component props designThe ButtonProps interface properly extends the native button attributes while adding the variant prop with clear type definitions. The comment about the size prop provides context for future development.
58-67: Well-structured conditional stylingThe conditional styling logic is well-organized, first applying variant styles, then handling disabled state, and finally adding hover and focus styles as appropriate.
71-83: Good implementation of button interactionsThe button correctly implements mouse and focus event handlers, and properly applies all the combined styles. The use of forwardRef is appropriate for allowing parent components to access the underlying button element.
apps/demo/components/KeepAliveDemo.tsx (1)
1-108: Well-structured keep-alive demonstration component!This is a good demonstration of the keep-alive feature. The component illustrates how a Cubit persists even when no components are using it, and how multiple components can share the same Cubit instance.
The comments explaining the behavior are comprehensive and the console logs provide good debugging insights into the component lifecycle.
apps/docs/learn/blac-pattern.md (1)
128-130: Clear explanation of unidirectional data flow.The updated explanation of how state updates occur via internal logic for Cubit and event handlers for Bloc is well-articulated and aligned with the new event-handler architecture.
Also applies to: 145-145
apps/demo/blocs/ConditionalUserProfileCubit.ts (2)
23-28: Good prevention of infinite loops in the getter.The comments explaining the potential for infinite loops when patching state inside a getter are valuable. This is a common pitfall when using getters with reactive state systems.
1-56: Well-structured Cubit implementation.This is a clean implementation of a Cubit for managing user profile state with conditional rendering support. The state interface is well-defined, and the methods for state manipulation are logical and consistent.
apps/demo/components/BlocToBlocCommsDemo.tsx (1)
1-79: Well-implemented demo of inter-bloc communication.This component effectively demonstrates communication between two different Bloc/Cubit instances. The UI is clear and the explanatory text helps users understand what's happening.
apps/demo/blocs/DashboardStatsCubit.ts (3)
1-14: Well-structured state interface and initialization.The interface design effectively captures the essential state properties for dashboard statistics, with proper typing and an appropriate initial state setup.
16-24: Good use of the isolated pattern.The
static isolated = trueproperty ensures that each instance maintains independent state, which is appropriate for this use case. The explanatory comments provide helpful context about the isolation behavior.
57-60: LGTM: Good state reset implementation.The
resetStatsmethod correctly usesemitto completely replace the state with the initial state object.apps/demo/components/MultiInstanceDemo.tsx (2)
6-21: Well-implemented counter component with proper bloc usage.The
CounterInstancecomponent correctly uses theuseBlochook with appropriate ID generation and props passing. Good use of destructuring to access state and cubit.
23-36: Good demonstration of instance isolation and state sharing.The component effectively demonstrates different instances of the same bloc with isolated state. The duplicate instances with ID "gamma" (lines 29-30) show how shared IDs result in shared state across components.
apps/demo/components/BlocWithReducerDemo.tsx (3)
38-63: Good use of useBloc hook with form handling.The component correctly implements the Todo form with proper state management and form submission handling.
64-75: LGTM: Using bloc.filteredTodos correctly.The code appropriately uses the derived
filteredTodosproperty from the bloc to render the filtered list of todos.
106-111: Great documentation in the UI about the event-handler pattern.The explanatory text does an excellent job of documenting the event-handler pattern used in the TodoBloc.
apps/docs/api/key-methods.md (5)
88-109: Clear and thorough documentation of the new event handling pattern.The documentation effectively explains the new
onmethod for registering event handlers, including its signature, parameters, and intended usage.
110-130: Good example of event handler registration.The example code clearly demonstrates how to define event classes and register handlers with the new pattern.
131-146: Clear documentation of the add method for event dispatching.The documentation effectively explains how the
addmethod dispatches event instances to their registered handlers.
149-197: Well-structured TodoBloc example showing the event pattern.The TodoBloc example effectively demonstrates the new event-class and handler registration approach with practical implementation details.
258-262: Updated action to event terminology in subscription example.Good update to use the term "event" instead of "action" to maintain consistency with the new event-class based approach.
packages/blac/src/BlocBase.ts (6)
6-6: Simplified the dependency selector type signature.The
DependencySelectortype definition has been simplified by removing the optional old state parameter. This streamlines the API and matches the actual usage pattern throughout the codebase.
122-122: Replaced event dispatching with logging.Event dispatching via lifecycle events has been replaced with direct logging, simplifying the system by removing the event dispatch mechanism for bloc creation.
167-171: Improved disposal mechanism.The changes improve bloc disposal by:
- Changing
dispose()toclear()to match updated API- Removing event dispatching in favor of direct logging
- Explicitly calling
Blac.instance.disposeBloc(this)for cleaner disposalThis simplifies the code path and makes the disposal process more explicit.
193-198: Added idempotent consumer management.Good improvement to make
_addConsumeridempotent by checking if the consumer already exists before adding it. This prevents duplicate entries and simplifies consumer handling logic.
207-212: Added idempotent consumer removal.Similar to the add operation, the
_removeConsumermethod now checks if the consumer exists before attempting to remove it. This makes the API more robust against potential misuse.
26-26:❓ Verification inconclusive
Added a unique identifier property.
The new
uidproperty provides a unique identifier for each Bloc instance usingcrypto.randomUUID(). This is useful for debugging and tracking bloc instances.Make sure
crypto.randomUUID()is available in all target environments:
🏁 Script executed:
#!/bin/bash # Check for browser compatibility of crypto.randomUUID() curl -s https://caniuse.com/api/features/mdn-api_crypto_randomuuid.json | jqLength of output: 78
🏁 Script executed:
#!/bin/bash # Check Node.js version and availability of crypto.randomUUID() node -v node -e "console.log('crypto.randomUUID type:', typeof crypto.randomUUID)"Length of output: 133
Verify
crypto.randomUUID()Support
Please confirm thatcrypto.randomUUID()is supported across all your target environments (especially browsers) or provide a fallback/polyfill for older runtimes.• File:
packages/blac/src/BlocBase.ts(line 26)
• Added:public uid = crypto.randomUUID();• Verified: Node v24+ supports
crypto.randomUUID()
• Action:
- Check browser compatibility in your target matrix (e.g., via MDN or Can I Use).
- If any targets lack support, consider using a small UUID library (e.g.,
uuid) or a custom polyfill.apps/docs/api/core-classes.md (6)
80-82: Updated documentation to reflect the shift to an event-handler pattern.The documentation has been updated to reflect the architectural change from an action-reducer pattern to an event-handler pattern. This shift provides a more intuitive API for handling complex state transitions.
87-87: Clarified the event type parameter.The documentation now correctly describes the
Etype parameter as representing event classes rather than action types, which aligns with the new architecture.
100-102: Updated method documentation to reflect the event-handler API.The method table now accurately describes the
onandaddmethods that are central to the event-handler pattern, clearly explaining their parameters and purpose.
106-138: Completely revised example to showcase event-handler pattern.The example has been thoroughly rewritten to demonstrate:
- How to define event classes
- How to register event handlers with the
onmethod- How to dispatch events with the
addmethod- How to create helper methods for common events
This provides clear guidance for users adopting the new pattern.
149-149: Updated BlacEvent description for Action.The documentation now correctly describes the Action event as being triggered by
bloc.add(), aligning with the new event-handler approach.
153-154: Revised guidance for choosing between Cubit and Bloc.The updated guidance clearly articulates when to use each class based on state complexity and architectural needs, helping users make informed decisions.
apps/demo/blocs/ComplexStateCubit.ts (5)
3-13: Well-structured complex state interface.The
ComplexDemoStateinterface is well-designed, incorporating various types of data (primitives, nested objects, arrays) to demonstrate managing complex state shapes.
15-28: Clear initialization with meaningful defaults.The constructor properly initializes all state properties with sensible default values, providing a good starting point for the demonstration.
30-47: Comprehensive state update methods.These methods demonstrate various state update patterns, including:
- Simple primitive updates
- Toggle operations
- Nested object updates with immutability
- Array modifications
All maintain proper immutability, which is essential for predictable state management.
48-58: Clear state reset functionality.The
resetStatemethod demonstrates the use ofemitto replace the entire state, which is appropriate for a complete reset operation.
60-67: Well-implemented computed getters.The getters demonstrate how to derive values from state without storing them directly in the state object, showcasing a clean separation of derived and stored state.
packages/blac/tests/BlacObserver.test.ts (5)
4-5: Added test event class.Good addition of a
DummyEventclass for testing purposes, which makes the tests more realistic and aligned with the new event-handler pattern.
7-12: Updated DummyBloc to use typed events.The
DummyBlochas been updated to use the proper event typing and includes a no-op handler registration to simulate the event handling architecture.
23-23: Updated observer collection check.Test now correctly uses
has()to check for observer presence in aSetrather thanincludes(), reflecting the change in the underlying data structure.
48-70: Enhanced notification test to verify event passing.The notification test has been significantly improved to:
- Create a proper test event
- Set it as the current action on the bloc
- Verify that observers receive the event along with state changes
- Clean up properly after the test
This thoroughly tests the observer notification system with the new event architecture.
83-83: Updated method name from dispose to clear.Test now calls
clear()instead ofdispose(), matching the renamed API method in the BlacObservable class.apps/demo/App.tsx (4)
1-30: Looks good - clean imports and style organization.The imports and style organization make for a clean, maintainable structure. The commented-out import on line 10 with explanation helps understand why simpler styling was chosen.
31-83: Well-structured reusable DemoCard component.The DemoCard component is well designed with:
- Clear prop typings
- Consistent styling using imported style constants
- Good separation of concerns between presentation and toggle functionality
- Proper conditional rendering of children
One small optimization could be to destructure the unused props in the function signature to make it clear they're available but intentionally not used.
85-101: Clean state management setup.The visibility state management is well-structured using a single state object with boolean flags. The default value approach is good for maintainability.
102-258: Consistent demo card implementation with comprehensive examples.The App component provides a comprehensive showcase of various Blac features through consistently structured demo cards. Each demo has clear titles and descriptions that explain what aspect of Blac they demonstrate.
apps/docs/api/react-hooks.md (4)
73-83: Great intro to conditional dependency tracking.This introduction clearly explains the intelligent dependency tracking capability of the
useBlochook, which is an important feature for optimizing rendering.
84-114: Well-structured example class definition.The
UserProfileCubitexample clearly demonstrates a cubit with multiple properties and a getter that has a side effect, which is perfect for demonstrating dependency tracking behaviors.
117-152: Clear component example with descriptive logging.The component example effectively demonstrates how to use the cubit with conditional rendering. The console log helps users understand when re-renders occur, which reinforces the explanation.
154-179: Excellent explanation of behavior with specific scenarios.The behavior explanation with numbered points effectively walks through different scenarios, making it clear how dependency tracking changes based on what state is actually used in the rendering.
apps/docs/learn/core-concepts.md (2)
82-112: Clear event class definitions and handler registrations.The example provides a clear demonstration of how to define event classes and register handlers using the new pattern. The constructor with event payload and optional helper methods are well-illustrated.
118-119: Updated state update explanation for clarity.The explanation of how state updates occur has been updated to reflect the new event-handler approach, which clearly explains the flow from
addto handlers to state updates.apps/docs/docs/api/core-classes.md (6)
80-83: Clear description of the updated Bloc pattern.The updated description correctly reflects the shift from an action/reducer pattern to an event-handler pattern, which is important for users to understand the architectural change.
87-88: Improved type parameter description.The description of the
Etype parameter more accurately reflects its purpose as event classes rather than plain action objects.
98-102: Well-defined method signatures for the event-handler pattern.The method descriptions for
onandaddclearly explain their roles in the new event-handler pattern, making it easy for users to understand how to register and dispatch events.
106-139: Comprehensive example demonstrating the event-handler pattern.The example effectively demonstrates:
- How to define event classes
- How to create a union type for events
- How to register handlers in the constructor
- How to create helper methods to dispatch events
This provides users with a complete reference implementation.
149-150: Updated BlacEvent.Action description to reflect new pattern.The description of the
Actionevent has been updated to refer to events being dispatched viabloc.add()rather than actions, which is consistent with the new terminology.
153-154: Clear guidance on choosing between Cubit and Bloc.The updated guidance clearly explains when to use each container type based on complexity and the need for event-driven state management.
apps/demo/components/CustomSelectorDemo.tsx (1)
47-62: Button label may still cause parent re-render, contradicting “NO RE-RENDER” note.
CustomSelectorDemoitself subscribes to the entire cubit state (useBloc(ComplexStateCubit)), so pressing “INC OTHER COUNT (NO RE-RENDER)” will still trigger a re-render of this parent component, even thoughCustomSelectorDisplayis unaffected.
If the intention is to prove that onlyCustomSelectorDisplayis stabilised, consider:• Moving the
anotherCounterbutton into its own memoised child, or
• UsinguseBlocwith aselectorthat omitsanotherCounterin the parent as well.This will make the demo’s behaviour match the label.
packages/blac/src/Blac.ts (1)
21-26: RenameddependencySelector→selector– double-check all call-sites
GetBlocOptionsnow exposesselector?: BlocHookDependencyArrayFn<…>.
Please ensure every invocation across@blac/react, demos, and docs has been updated; otherwise consumers may receive an “unknown property” TS error at compile time.
If you’d like help generating a search script to confirm, let me know!packages/blac/README.md (1)
50-72: Documentation update looks greatThe rewritten Bloc example clearly explains the new event-handler pattern and aligns with the shipped API. Nice work!
packages/blac-react/src/externalBlocStore.ts (1)
139-180:❌ Incorrect review comment
Observer leak risk –
ridnever unregistered
subscribehands anid: ridto_observer.subscribebut the returnedunSub()only removes the JS listener, not the record in Blac’s global observer map (if such map tracks IDs). Confirm whetherunSubcleans up the observer entry; otherwise duplicaterids across remounts will accumulate.If cleanup is missing, add inside the returned function:
Blac.instance._observer.unsubscribe(rid);Please verify with a quick search:
🏁 Script executed:
#!/bin/bash # Is there an unsubscribe implementation that accepts an ID? rg --context 3 "unsubscribe\\(.*rid" packages/blacLength of output: 51
🏁 Script executed:
#!/bin/bash # Search for the Blac class definition rg --context 2 "class Blac" packages/blac # Search for BlacObserver usage or definition rg --context 3 "BlacObserver" packages/blac # Inspect subscribe implementation in the observer rg --context 3 "subscribe(" packages/blac # Inspect any unsubscribe method rg --context 3 "unsubscribe(" packages/blacLength of output: 5916
No observer leak: subscription cleanup is sufficient
TheunSub()returned byBlacObservable.subscribe(observer)invokesunsubscribe(observer), which deletes the observer from the bloc’s_observersset. There is no separate global map keyed byid; theidfield is only metadata and doesn’t require extra teardown. You can safely ignore the leak concern.Likely an incorrect or invalid review comment.
packages/blac/tests/BlocBase.test.ts
Outdated
| // describe('handleUnsubscribe', () => { | ||
| // // This entire describe block is commented out as its tests were removed. | ||
| // // it('should report `listener_removed` when a listener is removed', () => { | ||
| // // // ... | ||
| // // }); | ||
| // }); | ||
|
|
||
| // describe('dispose', () => { | ||
| // // This entire describe block is commented out as its tests were removed. | ||
| // // it('should report `bloc_disposed` when disposed', () => { | ||
| // // // ... | ||
| // // }); | ||
| // }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid commented-out tests in favor of proper test maintenance.
Instead of commenting out entire test sections, consider:
- Completely removing these tests if the functionality is no longer supported
- Using
it.skip()ordescribe.skip()to properly mark the tests as skipped
Commented-out tests reduce visibility into test coverage and can lead to confusion for future developers.
- // describe('handleUnsubscribe', () => {
- // This entire describe block is commented out as its tests were removed.
- // it('should report `listener_removed` when a listener is removed', () => {
- // // ...
- // });
- // });
- // describe('dispose', () => {
- // This entire describe block is commented out as its tests were removed.
- // it('should report `bloc_disposed` when disposed', () => {
- // // ...
- // });
- // });If the lifecycle event functionality has been intentionally removed, these tests should be completely removed rather than commented out.
📝 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.
| // describe('handleUnsubscribe', () => { | |
| // // This entire describe block is commented out as its tests were removed. | |
| // // it('should report `listener_removed` when a listener is removed', () => { | |
| // // // ... | |
| // // }); | |
| // }); | |
| // describe('dispose', () => { | |
| // // This entire describe block is commented out as its tests were removed. | |
| // // it('should report `bloc_disposed` when disposed', () => { | |
| // // // ... | |
| // // }); | |
| // }); |
🤖 Prompt for AI Agents
In packages/blac/tests/BlocBase.test.ts between lines 73 and 85, there are
entire describe blocks commented out for tests related to handleUnsubscribe and
dispose. Instead of leaving these tests commented out, either fully remove these
test blocks if the related functionality is no longer supported, or convert them
to skipped tests using describe.skip() or it.skip() to clearly indicate they are
intentionally not run. This improves test clarity and maintenance.
| increment = () => { | ||
| this.patch({ count: this.state.count + 1 }); | ||
| console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} incremented to ${this.state.count +1}`); | ||
| }; |
There was a problem hiding this comment.
Fix the calculation error in the console.log statement.
The console.log statement is displaying this.state.count + 1, but at this point, the state has already been updated via this.patch(), so you should be using this.state.count directly.
increment = () => {
this.patch({ count: this.state.count + 1 });
- console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} incremented to ${this.state.count +1}`);
+ console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} incremented to ${this.state.count}`);
};📝 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.
| increment = () => { | |
| this.patch({ count: this.state.count + 1 }); | |
| console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} incremented to ${this.state.count +1}`); | |
| }; | |
| increment = () => { | |
| this.patch({ count: this.state.count + 1 }); | |
| console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} incremented to ${this.state.count}`); | |
| }; |
🤖 Prompt for AI Agents
In apps/demo/blocs/KeepAliveCounterCubit.ts around lines 19 to 22, the
console.log statement incorrectly adds 1 to this.state.count after the state has
already been updated by this.patch(). Fix this by logging this.state.count
directly without adding 1, so the output reflects the updated count accurately.
| // Linter has issues with onDispose override, so we'll skip it. | ||
| // If it worked, it would look like this: | ||
| // protected onDispose() { | ||
| // super.onDispose(); // Important to call super | ||
| // console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} DISPOSED.`); | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resolve the linter issues with onDispose instead of commenting it out.
Rather than leaving a commented implementation, consider:
- Fixing the linter issues by configuring the linter correctly
- Implementing the method properly with linter directives if needed
- Remove the commented code if it's not needed
Commented code reduces readability and may confuse future developers.
- // Linter has issues with onDispose override, so we'll skip it.
- // If it worked, it would look like this:
- // protected onDispose() {
- // super.onDispose(); // Important to call super
- // console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} DISPOSED.`);
- // }
+ // If you need to implement dispose functionality:
+ protected override onDispose(): void {
+ super.onDispose(); // Important to call super
+ console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} DISPOSED.`);
+ }If this is for a demo and you prefer to keep the console.log statements for debugging, consider adding a config flag to enable/disable logging.
📝 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.
| // Linter has issues with onDispose override, so we'll skip it. | |
| // If it worked, it would look like this: | |
| // protected onDispose() { | |
| // super.onDispose(); // Important to call super | |
| // console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} DISPOSED.`); | |
| // } | |
| // If you need to implement dispose functionality: | |
| protected override onDispose(): void { | |
| super.onDispose(); // Important to call super | |
| console.log(`KeepAliveCounterCubit instance ${this.state.instanceId} DISPOSED.`); | |
| } |
🤖 Prompt for AI Agents
In apps/demo/blocs/KeepAliveCounterCubit.ts around lines 30 to 35, the onDispose
method is commented out due to linter issues. To fix this, properly implement
the onDispose method by overriding it and calling super.onDispose(), then add
the console.log statement as needed. Address the linter errors by adjusting the
linter configuration or using inline linter directives to allow this override.
Finally, remove the commented-out code to improve readability and optionally add
a config flag to toggle the console logging for debugging purposes.
| @@ -0,0 +1,59 @@ | |||
| import { Cubit, Persist } from '@blac/core'; | |||
There was a problem hiding this comment.
The Persist addon appears to have been removed from the core package
According to the provided summary, the Persist addon has been removed from the core package, but this code still imports it from '@blac/core'. This will likely cause runtime errors.
#!/bin/bash
# Check if Persist is still exported from the @blac/core package
rg -A 5 "export.*Persist" --type ts packages/blac/🤖 Prompt for AI Agents
In apps/demo/blocs/PersistentSettingsCubit.ts at line 1, the import statement
includes Persist from '@blac/core', but Persist has been removed from that
package. Remove Persist from the import statement and update the code to import
Persist from its new location or remove its usage if no longer applicable to
prevent runtime errors.
| static addons = [ | ||
| Persist({ | ||
| keyName: 'demoAppSettings', // The key used in localStorage | ||
| defaultValue: { | ||
| theme: 'light', | ||
| notificationsEnabled: true, | ||
| userName: 'Guest', | ||
| }, | ||
| // initialState: initialSettings, // The addon can take an initial state too, useful if Cubit's super(initialState) is different or complex | ||
| // storage: sessionStorage, // To use sessionStorage instead of localStorage (default) | ||
| // serialize: (state) => JSON.stringify(state), // Custom serialize function | ||
| // deserialize: (jsonString) => JSON.parse(jsonString), // Custom deserialize function | ||
| }), | ||
| ]; |
There was a problem hiding this comment.
Persist addon usage may be invalid after core package changes
The static addons property using the Persist addon may no longer be supported based on the summary stating that "the entire addon system and plugin lifecycle events were deleted from the core packages/blac package".
#!/bin/bash
# Check if the Cubit class still supports the 'addons' static property
rg -A 10 "class Cubit" --type ts packages/blac/🤖 Prompt for AI Agents
In apps/demo/blocs/PersistentSettingsCubit.ts around lines 18 to 31, the static
addons property using the Persist addon is likely no longer supported due to
removal of the addon system in the core package. Remove the static addons
property and refactor the persistence logic to use the new recommended approach
for state persistence, such as manually handling localStorage or using updated
APIs provided by the core package. Verify the current Cubit class implementation
and adapt the persistence mechanism accordingly.
| const getBloc = useCallback(() => { | ||
| return Blac.getBloc(bloc, { | ||
| id: effectiveBlocId, | ||
| props, | ||
| instanceRef: rid | ||
| }); | ||
| }, [bloc, props]); |
There was a problem hiding this comment.
Stale id / isolation logic due to missing dependencies
useCallback only lists bloc and props as dependencies, yet effectiveBlocId (derived from blocId & rid) is captured in the closure.
If the caller changes options.id, getBloc will still use the initial id, producing wrong Bloc instances and breaking memoization caching.
- }, [bloc, props]);
+ }, [bloc, props, effectiveBlocId]);(You can safely drop rid because it is stable thanks to the empty ‑array memo above.)
📝 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.
| const getBloc = useCallback(() => { | |
| return Blac.getBloc(bloc, { | |
| id: effectiveBlocId, | |
| props, | |
| instanceRef: rid | |
| }); | |
| }, [bloc, props]); | |
| const getBloc = useCallback(() => { | |
| return Blac.getBloc(bloc, { | |
| id: effectiveBlocId, | |
| props, | |
| instanceRef: rid | |
| }); | |
| }, [bloc, props, effectiveBlocId]); |
🤖 Prompt for AI Agents
In packages/blac-react/src/externalBlocStore.ts around lines 70 to 76, the
useCallback hook for getBloc is missing effectiveBlocId in its dependency array,
causing stale id usage and incorrect Bloc instances. To fix this, add
effectiveBlocId to the dependency array of useCallback and remove rid since it
is stable, ensuring getBloc updates correctly when the id changes.
| const blocInstance = useRef<InstanceType<B>>(getBloc()); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
blocInstance never updates when id or props change
useRef(getBloc()) is executed only once. If callers render the same hook with a different id or constructor props, the stored instance becomes out-of-sync.
- const blocInstance = useRef<InstanceType<B>>(getBloc());
+ const blocInstance = useRef<InstanceType<B>>(getBloc());
+
+ // Ensure the ref is kept in sync with id/prop changes
+ const nextInstance = getBloc();
+ if (blocInstance.current !== nextInstance) {
+ blocInstance.current = nextInstance;
+ }This keeps the ref correct without forcing a re-render.
📝 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.
| const blocInstance = useRef<InstanceType<B>>(getBloc()); | |
| const blocInstance = useRef<InstanceType<B>>(getBloc()); | |
| // Ensure the ref is kept in sync with id/prop changes | |
| const nextInstance = getBloc(); | |
| if (blocInstance.current !== nextInstance) { | |
| blocInstance.current = nextInstance; | |
| } |
🤖 Prompt for AI Agents
In packages/blac-react/src/externalBlocStore.ts around lines 78 to 79, the
useRef hook is initialized with getBloc() only once, so blocInstance does not
update when id or props change, causing it to become out-of-sync. To fix this,
replace useRef with useMemo or useState to recreate the blocInstance whenever id
or props change, ensuring the stored instance stays current without triggering
unnecessary re-renders.
| const rid = useMemo(() => { | ||
| return crypto.randomUUID(); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
crypto.randomUUID() may break in non-browser environments
crypto.randomUUID() is only available in modern browsers / Node ≥ 19. If this hook is ever rendered during SSR or inside older Node versions the call will throw and crash the render.
- const rid = useMemo(() => {
- return crypto.randomUUID();
- }, []);
+ const rid = useMemo(() => {
+ // Fall back to a RFC-4122 polyfill when `crypto.randomUUID` is unavailable
+ return (typeof crypto !== 'undefined' && crypto.randomUUID)
+ ? crypto.randomUUID()
+ : ([1e7]+-1e3+-4e3+-8e3+-1e11)
+ .replace(/[018]/g,(c:any)=>
+ (c ^ (Math.random()*16>>c/4)).toString(16));
+ }, []);This keeps the hook universal (browser, React-Native, Node/SSR).
📝 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.
| const rid = useMemo(() => { | |
| return crypto.randomUUID(); | |
| }, []); | |
| const rid = useMemo(() => { | |
| // Fall back to a RFC-4122 polyfill when `crypto.randomUUID` is unavailable | |
| return (typeof crypto !== 'undefined' && crypto.randomUUID) | |
| ? crypto.randomUUID() | |
| : ([1e7]+-1e3+-4e3+-8e3+-1e11) | |
| .replace(/[018]/g,(c:any)=> | |
| (c ^ (Math.random()*16>>c/4)).toString(16)); | |
| }, []); |
🤖 Prompt for AI Agents
In packages/blac-react/src/externalBlocStore.ts around lines 58 to 60, the use
of crypto.randomUUID() can cause crashes in non-browser or older Node
environments due to lack of support. Replace this call with a universal UUID
generation method or a polyfill that works across browsers, React Native, and
Node/SSR environments to ensure the hook does not break during server-side
rendering or in older runtimes.
| const dependencyArray: BlocHookDependencyArrayFn<BlocState<InstanceType<B>>> = | ||
| useMemo( | ||
| () => | ||
| (newState): ReturnType<BlocHookDependencyArrayFn<BlocState<InstanceType<B>>>> => { | ||
| const instance = blocInstance.current; | ||
|
|
||
| if (!instance) { | ||
| return []; | ||
| } | ||
|
|
||
| // Use custom dependency selector if provided | ||
| if (selector) { | ||
| return selector(newState); | ||
| } | ||
|
|
||
| // Fall back to bloc's default dependency selector if available | ||
| if (instance.defaultDependencySelector) { | ||
| return instance.defaultDependencySelector(newState); | ||
| } | ||
|
|
||
| // For primitive states, use default selector | ||
| if (typeof newState !== 'object') { | ||
| // Default behavior for primitive states: re-render if the state itself changes. | ||
| return [[newState]]; | ||
| } | ||
|
|
||
| // For object states, track which properties were actually used | ||
| const usedStateValues: string[] = []; | ||
| for (const key of usedKeys.current) { | ||
| if (key in newState) { | ||
| usedStateValues.push(newState[key as keyof typeof newState]); | ||
| } | ||
| } | ||
|
|
||
| // Track used class properties for dependency tracking, this enables rerenders when class getters change | ||
| const usedClassValues: unknown[] = []; | ||
| for (const key of usedClassPropKeys.current) { | ||
| if (key in instance) { | ||
| try { | ||
| const value = instance[key as keyof InstanceType<B>]; | ||
| switch (typeof value) { | ||
| case 'function': | ||
| continue; | ||
| default: | ||
| usedClassValues.push(value); | ||
| continue; | ||
| } | ||
| } catch (error) { | ||
| Blac.instance.log('useBloc Error', error); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return [usedStateValues, usedClassValues]; | ||
| }, | ||
| // Pass the dependency array to control when the subscription is updated | ||
| dependencyArray, | ||
| // Use the provided id to identify this subscription | ||
| id: rid, | ||
| }); | ||
|
|
||
| // Return an unsubscribe function that can be called to clean up the subscription | ||
| return () => { | ||
| unSub(); | ||
| }; | ||
| }, | ||
| // Return an immutable snapshot of the current bloc state | ||
| getSnapshot: (): BlocState<B> => asBlocBase.state, | ||
| // Server snapshot mirrors the client snapshot in this implementation | ||
| getServerSnapshot: (): BlocState<B> => asBlocBase.state, | ||
| [], | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Dependency selector memo loses updates & has type issue
useMemo(..., [])freezesselectorandinstance.defaultDependencySelectorcaptured from the first render. Prop-driven changes toselectorwill therefore be ignored.usedStateValuesis declaredstring[]but you later push any property value, causing a TS error / unsafe cast.
- const dependencyArray: BlocHookDependencyArrayFn<BlocState<InstanceType<B>>> =
- useMemo(
- () =>
+ const dependencyArray: BlocHookDependencyArrayFn<
+ BlocState<InstanceType<B>>
+ > = useMemo(
+ () =>
(newState) => {
...
- const usedStateValues: string[] = [];
+ const usedStateValues: unknown[] = [];
...
return [usedStateValues, usedClassValues];
},
- [],
+ [selector, blocInstance] // refresh when selector or instance changes
);This fixes both runtime correctness and TypeScript accuracy.
📝 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.
| const dependencyArray: BlocHookDependencyArrayFn<BlocState<InstanceType<B>>> = | |
| useMemo( | |
| () => | |
| (newState): ReturnType<BlocHookDependencyArrayFn<BlocState<InstanceType<B>>>> => { | |
| const instance = blocInstance.current; | |
| if (!instance) { | |
| return []; | |
| } | |
| // Use custom dependency selector if provided | |
| if (selector) { | |
| return selector(newState); | |
| } | |
| // Fall back to bloc's default dependency selector if available | |
| if (instance.defaultDependencySelector) { | |
| return instance.defaultDependencySelector(newState); | |
| } | |
| // For primitive states, use default selector | |
| if (typeof newState !== 'object') { | |
| // Default behavior for primitive states: re-render if the state itself changes. | |
| return [[newState]]; | |
| } | |
| // For object states, track which properties were actually used | |
| const usedStateValues: string[] = []; | |
| for (const key of usedKeys.current) { | |
| if (key in newState) { | |
| usedStateValues.push(newState[key as keyof typeof newState]); | |
| } | |
| } | |
| // Track used class properties for dependency tracking, this enables rerenders when class getters change | |
| const usedClassValues: unknown[] = []; | |
| for (const key of usedClassPropKeys.current) { | |
| if (key in instance) { | |
| try { | |
| const value = instance[key as keyof InstanceType<B>]; | |
| switch (typeof value) { | |
| case 'function': | |
| continue; | |
| default: | |
| usedClassValues.push(value); | |
| continue; | |
| } | |
| } catch (error) { | |
| Blac.instance.log('useBloc Error', error); | |
| } | |
| } | |
| } | |
| return [usedStateValues, usedClassValues]; | |
| }, | |
| // Pass the dependency array to control when the subscription is updated | |
| dependencyArray, | |
| // Use the provided id to identify this subscription | |
| id: rid, | |
| }); | |
| // Return an unsubscribe function that can be called to clean up the subscription | |
| return () => { | |
| unSub(); | |
| }; | |
| }, | |
| // Return an immutable snapshot of the current bloc state | |
| getSnapshot: (): BlocState<B> => asBlocBase.state, | |
| // Server snapshot mirrors the client snapshot in this implementation | |
| getServerSnapshot: (): BlocState<B> => asBlocBase.state, | |
| [], | |
| ); | |
| const dependencyArray: BlocHookDependencyArrayFn< | |
| BlocState<InstanceType<B>> | |
| > = useMemo( | |
| () => | |
| (newState): ReturnType<BlocHookDependencyArrayFn<BlocState<InstanceType<B>>>> => { | |
| const instance = blocInstance.current; | |
| if (!instance) { | |
| return []; | |
| } | |
| // Use custom dependency selector if provided | |
| if (selector) { | |
| return selector(newState); | |
| } | |
| // Fall back to bloc's default dependency selector if available | |
| if (instance.defaultDependencySelector) { | |
| return instance.defaultDependencySelector(newState); | |
| } | |
| // For primitive states, use default selector | |
| if (typeof newState !== 'object') { | |
| // Default behavior for primitive states: re-render if the state itself changes. | |
| return [[newState]]; | |
| } | |
| // For object states, track which properties were actually used | |
| const usedStateValues: unknown[] = []; | |
| for (const key of usedKeys.current) { | |
| if (key in newState) { | |
| usedStateValues.push(newState[key as keyof typeof newState]); | |
| } | |
| } | |
| // Track used class properties for dependency tracking, this enables rerenders when class getters change | |
| const usedClassValues: unknown[] = []; | |
| for (const key of usedClassPropKeys.current) { | |
| if (key in instance) { | |
| try { | |
| const value = instance[key as keyof InstanceType<B>]; | |
| switch (typeof value) { | |
| case 'function': | |
| continue; | |
| default: | |
| usedClassValues.push(value); | |
| continue; | |
| } | |
| } catch (error) { | |
| Blac.instance.log('useBloc Error', error); | |
| } | |
| } | |
| } | |
| return [usedStateValues, usedClassValues]; | |
| }, | |
| [selector, blocInstance], // refresh when selector or instance changes | |
| ); |
🤖 Prompt for AI Agents
In packages/blac-react/src/externalBlocStore.ts between lines 81 and 137, the
useMemo hook has an empty dependency array causing it to capture the initial
selector and instance.defaultDependencySelector only once, ignoring updates.
Also, usedStateValues is typed as string[] but stores any property value,
causing a type error. Fix this by adding selector and
instance.defaultDependencySelector to the useMemo dependency array to ensure
updates are captured, and change the type of usedStateValues to any[] to
correctly reflect the stored values.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/blac-react/src/useBloc.tsx (1)
42-44: Remove the leftoverloghelper (duplicate from previous review).This debug helper is still shipped but never invoked, increasing bundle size and risking accidental console leakage.
Same remark was raised in an earlier review – please delete it.-const log = (...args: unknown[]) => { - console.log('useBloc', ...args); -};
🧹 Nitpick comments (2)
packages/blac-react/src/useExternalBlocStore.ts (2)
132-174:externalStoreis frozen to first render – props likedependencyArraywon’t update.Because the entire external store object is wrapped in
useMemowith[], any later changes to:
dependencyArray(see previous comment),blocInstance(e.g. newid/props),are ignored. Either drop the
useMemo(object is cheap) or include the relevant deps.-}, []); +}, [dependencyArray, blocInstance.current]);
141-150: Prefer library logger overconsole.errorfor uniform diagnostics.A direct
console.errorbypasses the Blac logging strategy. Forward throughBlac.log(or whichever method exists) so logs can be filtered or silenced in production.- console.error({ e, blocInstance, dependencyArray }); + Blac.log?.('useExternalBlocStore listener error', { + error: e, + blocInstance: blocInstance.current, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
packages/blac-react/src/index.ts(1 hunks)packages/blac-react/src/useBloc.tsx(4 hunks)packages/blac-react/src/useExternalBlocStore.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/blac-react/src/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/blac-react/src/useExternalBlocStore.ts (6)
packages/blac/src/types.ts (4)
BlocConstructor(23-23)BlocState(36-36)BlocBaseAbstract(15-17)BlocHookDependencyArrayFn(60-62)packages/blac/src/BlocBase.ts (2)
state(136-138)isIsolated(33-35)packages/blac-react/src/index.ts (1)
useExternalBlocStore(4-4)packages/blac-react/src/useBloc.tsx (1)
BlocHookOptions(27-33)packages/blac/src/Blac.ts (1)
Blac(38-416)packages/blac/src/BlacObserver.ts (1)
BlacObserver(9-20)
🔇 Additional comments (3)
packages/blac-react/src/useBloc.tsx (1)
117-132: 🛠️ Refactor suggestion
useEffectfires on every render – consumer bookkeeping will multiply.
instance.current(a mutableref) is embedded directly in the effect’s dependency list.
Because the value ofref.currentchanges on every state transition, React treats it as a new dependency and re-runs the effect, repeatedly calling:
instance.current._addConsumer(rid)- (on cleanup)
instance.current._removeConsumer(rid)This can:
- Add / remove the same consumer on every render – needless work.
- Cause duplicate registrations if
_addConsumerdoes not guard against it.- Introduce subtle memory leaks if the cleanup ever fails.
Run the effect once per mounted component by depending only on the stable
rid(and optionallyblocif the constructor may change), and capture the ref inside the effect:-useEffect(() => { - instance.current._addConsumer(rid); +useEffect(() => { + const inst = instance.current; + if (!inst) return; + + inst._addConsumer(rid); ... -}, [instance.current, rid]); +}, [rid]);Likely an incorrect or invalid review comment.
packages/blac-react/src/useExternalBlocStore.ts (2)
51-54:crypto.randomUUID()may not exist in all runtimes.Older Node versions (<19) and some browsers/polyfills lack
crypto.randomUUID. Falling back touuidor a small custom shim avoids runtime crashes.-const rid = useMemo(() => { - return crypto.randomUUID(); -}, []); +const rid = useMemo(() => { + return typeof crypto?.randomUUID === 'function' + ? crypto.randomUUID() + : Math.random().toString(36).slice(2); +}, []);
121-124:Blac.instance.logmight be undefined.
Blacin core exports static utilities;Blac.instanceis not documented elsewhere.
CallingBlac.instance.logrisks a runtimeundefinedaccess. Verify the correct logging API.
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the demo application to version 2.0.0-rc-3, adds a comprehensive set of React demo components illustrating advanced state-management patterns with the Blac library, and refactors core Blac classes to use explicit event-handler registration while removing deprecated plugin and lifecycle plumbing.
- Added multiple React demo components (custom selectors, conditional dependencies, reducer-based Bloc, Bloc-to-Bloc communication, keep-alive, etc.).
- Refactored core Bloc/Cubit classes to the new event-handler pattern and removed plugin/lifecycle dispatching.
- Updated UI components and styling for consistency, and improved documentation in comments.
Reviewed Changes
Copilot reviewed 75 out of 75 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/demo/components/CustomSelectorDemo.tsx | New demo showing fine-grained re-render control via custom selector. |
| apps/demo/components/ConditionalDependencyDemo.tsx | Demo illustrating conditional dependency tracking. |
| apps/demo/components/BlocWithReducerDemo.tsx | Todo list demo using Bloc with explicit event handlers. |
| apps/demo/components/BlocToBlocCommsDemo.tsx | Demo of inter-Bloc communication (Auth ⇄ DashboardStats). |
| apps/demo/components/BasicCounterDemo.tsx | Simple shared counter demo using CounterCubit. |
| apps/demo/components/.gitkeep | Placeholder to ensure directory is tracked. |
| apps/demo/blocs/UserProfileBloc.ts | New isolated UserProfileBloc with props and getters. |
| apps/demo/blocs/TodoBloc.ts | Bloc implementing todo actions with reducer handlers. |
| apps/demo/blocs/PersistentSettingsCubit.ts | Settings cubit configured with Persist addon. |
| apps/demo/blocs/LoggerEventCubit.ts | Cubit to demonstrate event subscriptions. |
| apps/demo/blocs/LifecycleCubit.ts | Lifecycle-aware cubit for mount/unmount demos. |
| apps/demo/blocs/KeepAliveCounterCubit.ts | Keep-alive cubit preserving instance across unmounts. |
| apps/demo/blocs/DashboardStatsCubit.ts | Isolated cubit accessing a shared AuthCubit. |
| apps/demo/blocs/CounterCubit.ts | Shared and isolated counter cubit definitions. |
| apps/demo/blocs/ConditionalUserProfileCubit.ts | Cubit showing conditional full-name rendering. |
| apps/demo/blocs/ComplexStateCubit.ts | Complex state cubit with multiple getters. |
| apps/demo/blocs/AuthCubit.ts | Shared authentication cubit with async login/logout. |
| apps/demo/blocs/.gitkeep | Placeholder to ensure directory is tracked. |
| apps/demo/LcarsHeader.tsx | LCARS‐style header component with date/time updates. |
| apps/demo/App.tsx | Main application wiring together all demo cards. |
Comments suppressed due to low confidence (1)
apps/demo/components/BlocWithReducerDemo.tsx:21
- Using a clickable
<span>for toggling may not be accessible—switch to a<button>element or addrole="button"and keyboard event handlers for proper ARIA support.
<span onClick={() => onToggle(todo.id)}
| const [authState, authCubit] = useBloc(AuthCubit); | ||
| // DashboardStatsCubit is isolated (static isolated = true) | ||
| const [dashboardState, dashboardStatsCubit] = useBloc(DashboardStatsCubit); | ||
| console.log('dashboardState', dashboardStatsCubit); |
There was a problem hiding this comment.
This debug console.log logs the cubit instance rather than its state and may be leftover from development—consider removing or revising it.
| console.log('dashboardState', dashboardStatsCubit); | |
| console.log('dashboardState', dashboardState); |
apps/demo/LcarsHeader.tsx
Outdated
|
|
||
| const LcarsHeaderButtonBlock: React.FC<LcarsHeaderButtonBlockProps> = ({ text, colorClass, align, onClick }) => { | ||
| return ( | ||
| <div className={`lcars-header-button-block ${colorClass} ${align === 'left' ? 'left-rounded' : 'right-rounded'}`} onClick={onClick}> |
There was a problem hiding this comment.
Clickable div should include role="button", tabIndex={0}, and handle onKeyPress to ensure keyboard accessibility.
| <div className={`lcars-header-button-block ${colorClass} ${align === 'left' ? 'left-rounded' : 'right-rounded'}`} onClick={onClick}> | |
| <div | |
| className={`lcars-header-button-block ${colorClass} ${align === 'left' ? 'left-rounded' : 'right-rounded'}`} | |
| role="button" | |
| tabIndex={0} | |
| onClick={onClick} | |
| onKeyPress={(e) => { | |
| if (onClick && (e.key === 'Enter' || e.key === ' ')) { | |
| e.preventDefault(); | |
| onClick(); | |
| } | |
| }} | |
| > |
apps/demo/App.tsx
Outdated
| title="Basic Shared Counter" | ||
| description="Demonstrates useBloc with a shared Cubit instance." | ||
| show={show.basic} | ||
| setShow={() => setShow({ ...show, basic: !show.basic })} |
There was a problem hiding this comment.
[nitpick] The DemoCard prop setShow is typed to accept a boolean, but here you're passing a zero-arg function that ignores that parameter—consider aligning the signature so setShow always takes the new visibility boolean.
| setShow={() => setShow({ ...show, basic: !show.basic })} | |
| setShow={(newShow) => setShow({ ...show, basic: newShow })} |
apps/demo/blocs/CounterCubit.ts
Outdated
| // Example of an inherently isolated version if needed directly | ||
| export class IsolatedCounterCubit extends Cubit<CounterState, CounterCubitProps> { | ||
| static isolated = true; | ||
|
|
||
| constructor(props?: CounterCubitProps) { | ||
| super({ count: props?.initialCount ?? 0 }); | ||
| } | ||
|
|
||
| increment = () => { | ||
| this.patch({ count: this.state.count + 1 }); | ||
| }; | ||
|
|
||
| decrement = () => { | ||
| this.patch({ count: this.state.count - 1 }); | ||
| }; | ||
| } No newline at end of file |
There was a problem hiding this comment.
[nitpick] The shared and isolated counter cubits duplicate the same logic—consider refactoring common methods into a base class to adhere to DRY principles.
| // Example of an inherently isolated version if needed directly | |
| export class IsolatedCounterCubit extends Cubit<CounterState, CounterCubitProps> { | |
| static isolated = true; | |
| constructor(props?: CounterCubitProps) { | |
| super({ count: props?.initialCount ?? 0 }); | |
| } | |
| increment = () => { | |
| this.patch({ count: this.state.count + 1 }); | |
| }; | |
| decrement = () => { | |
| this.patch({ count: this.state.count - 1 }); | |
| }; | |
| } | |
| export class CounterCubit extends BaseCounterCubit {} | |
| export class IsolatedCounterCubit extends BaseCounterCubit { | |
| static isolated = true; | |
| } |
| // The Persist addon will attempt to load from localStorage first. | ||
| // If not found, or if loading fails, it will use this initial state. | ||
| super(initialSettings); | ||
| console.log('PersistentSettingsCubit CONSTRUCTED. Initial state (after potential load):', this.state); |
There was a problem hiding this comment.
This startup console.log may clutter logs in production—consider removing or guarding it behind a debug flag.
| console.log('PersistentSettingsCubit CONSTRUCTED. Initial state (after potential load):', this.state); | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.log('PersistentSettingsCubit CONSTRUCTED. Initial state (after potential load):', this.state); | |
| } |
…ence • Fixed "Cannot access 'Blac' before initialization" circular dependency error • Implemented lazy initialization in SingletonBlacManager • Converted static property assignments to static getters • Enhanced useBloc type constraints from unknown to any for better inference • Added strategic type assertions to resolve TypeScript strictness issues • Updated documentation with troubleshooting guides and architecture notes • Improved proxy caching and lifecycle management in useBloc hook
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/blac-react/tests/useExternalBlocStore.edgeCases.test.tsx (1)
298-307: Same race-condition risk as main suite – await the promisesMirror the fix suggested in the main test file to guarantee all increments complete before the assertion.
packages/blac-react/src/useBloc.tsx (1)
43-45: Remove the unusedloghelper (repeat finding).
log()is still present but never invoked, re-introducing dead code the previous review already flagged. Please drop it before publishing.
🧹 Nitpick comments (21)
BLAC_IMPROVEMENTS_REVIEW.md (10)
1-7: Add document metadata for versioning and authorship
Consider including a date, version number, and author/contributor list at the top to track updates over time and clarify ownership.
9-24: Ensure consistent bullet indentation and formatting
Some nested lists under “Solutions Implemented” vary in indent level. Standardizing the indentation (e.g., two spaces per level) will improve readability.
29-42: Clarify file modification references
The “Files Modified” entries list large file ranges. Consider linking to specific PR diffs or sections to make it easier for readers to jump directly to the relevant changes.
61-73: Quantify performance gains with real metrics
You’ve outlined O(n)→O(1) improvements and proxy caching benefits. Adding before/after benchmarks or representative timings will make these claims more actionable.
78-93: Link architectural changes to code examples
The new dependency-injection pattern and manager interfaces are described but could benefit from inline code snippets or links to the implemented interfaces for quick reference.
94-109: Include test coverage statistics
The “Comprehensive Testing” section lists new test files and scenarios. Consider adding overall coverage percentages or badge to highlight coverage improvements.
153-161: Align table columns and precision
In the performance table, ensure uniform column widths and consistent formatting of improvement figures (e.g., “10–100×” vs “~95%”). This will improve scanability.
178-186: Link TypeScript warnings to issue tracker
For the remaining compilation warnings, consider adding issue IDs or GitHub links so maintainers can track resolutions in upcoming sprints.
207-216: Specify target dates for pending checklist items
The verification checklist marks two items as pending; adding expected completion dates or responsible owners will clarify next steps.
242-246: Add “Next Steps” subsection to conclusion
The conclusion summarizes successes well. Consider appending a brief “Next Steps” bullet list to guide future work or post-merge actions.review.md (1)
33-38: Replace bold “pseudo-headings” with real Markdown headingsMarkdown-lint (MD036) flags the repeated use of bold text (
**Title**) where a heading (#### Title) is expected.
Converting these lines to proper####sub-headings restores semantic structure and silences the linter.Example fix:
-**Fixed Circular Dependency Initialization** +#### Fixed Circular Dependency InitializationApply the same pattern to the other emphasised section titles.
Also applies to: 47-54, 64-76, 98-104, 112-138
BLAC_OVERVIEW.md (3)
88-89: Add missing “to” for grammatical correctness
Validate consumers periodically …→to validate consumers periodically …
127-129: Prefer “prefer … to …” phrasing“Prefer event-driven patterns over direct method calls” → “Prefer event-driven patterns to direct method calls”.
136-144: Tighten duplicated wordingLine 140 currently duplicates “state” – consider:
- **Performance Issues**: Accessing unnecessary properties causes extra renderspackages/blac-react/tests/useExternalBlocStore.test.tsx (2)
323-328: Tests reach into private internals
_dispose()and_consumersare private implementation details. Coupling tests to them reduces refactor freedom.
Consider exposing a publicdispose()/getConsumerCount()utility onBlocBaseand updating the tests accordingly.Also applies to: 400-408
346-353: Heavy 1 000-iteration loop slows the suiteCalling
increment()1 000 times and asserting 1 000 listener calls adds ~100 ms per run and little extra confidence over, say, 100.
Trim the loop to keep CI fast.packages/blac-react/tests/useExternalBlocStore.edgeCases.test.tsx (2)
346-353: Reduce 1 000-increment stress testA smaller count (100-200) still surfaces memory issues while shortening execution time.
65-73: Test manipulates_pushStatevia type-castDeliberately invoking internal
_pushStatethrough(this as any)hard-wires the test to internals.
If the goal is to assert runtime validation, expose a sanctioned helper or public API instead.CLAUDE.md (1)
84-86: Remove duplicated word“component integration tests” appears twice. Delete the repeat.
packages/blac-react/src/useBloc.tsx (1)
72-74: Delete commented-out debugging lines.The leftover
// const base …block adds noise and drifts out-of-date quickly. Prune it or move the logic into git history.- // const base = bloc as unknown as BlocBaseAbstract; - // const isIsolated = base.isolated; - // const effectiveBlocId = isIsolated ? rid : blocId;packages/blac/src/BlocBase.ts (1)
185-190:_consumerRefsis written but never read.The
WeakSettracking consumer refs is populated in_addConsumeryet unused elsewhere, adding overhead without benefit. Either integrate it intovalidateConsumersor remove the field & associated writes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
BLAC_IMPROVEMENTS_REVIEW.md(1 hunks)BLAC_OVERVIEW.md(1 hunks)CLAUDE.md(1 hunks)apps/demo/App.tsx(1 hunks)apps/demo/blocs/UserProfileBloc.ts(1 hunks)apps/docs/learn/architecture.md(1 hunks)apps/docs/learn/getting-started.md(2 hunks)packages/blac-react/package.json(2 hunks)packages/blac-react/src/useBloc.tsx(4 hunks)packages/blac-react/src/useExternalBlocStore.ts(1 hunks)packages/blac-react/tests/useExternalBlocStore.edgeCases.test.tsx(1 hunks)packages/blac-react/tests/useExternalBlocStore.test.tsx(1 hunks)packages/blac/package.json(2 hunks)packages/blac/src/Blac.ts(18 hunks)packages/blac/src/BlocBase.ts(7 hunks)review.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/docs/learn/architecture.md
- apps/demo/App.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/blac/package.json
- packages/blac-react/package.json
- apps/docs/learn/getting-started.md
- apps/demo/blocs/UserProfileBloc.ts
- packages/blac-react/src/useExternalBlocStore.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/blac-react/tests/useExternalBlocStore.edgeCases.test.tsx (4)
apps/demo/blocs/ComplexStateCubit.ts (1)
ComplexStateCubit(15-68)packages/blac/src/Blac.ts (2)
Blac(89-607)instance(91-93)packages/blac/src/BlocBase.ts (1)
state(129-131)packages/blac/src/BlacObserver.ts (1)
unsubscribe(80-88)
🪛 LanguageTool
BLAC_IMPROVEMENTS_REVIEW.md
[misspelling] ~80-~80: This word is normally spelled as one.
Context: ...actoring ✅ Issue: Global singleton anti-pattern and circular dependencies made testing ...
(EN_COMPOUNDS_ANTI_PATTERN)
BLAC_OVERVIEW.md
[duplication] ~16-~16: Possible typo: you repeated a word.
Context: ...anup and disposal mechanisms ### @blac/react React integration layer providing: - **useBlo...
(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~88-~88: Did you mean “to Validate”?
Context: ...Blocs()for bulk cleanup when needed - Validate consumers periodically withBlac.valid...
(MISSING_TO_BEFORE_A_VERB)
[grammar] ~128-~128: Consider using “to” with “prefer”.
Context: ... container-to-container communication - Prefer event-driven patterns over direct method calls - Keep coupling loose between dif...
(PREFER_OVER_TO)
[uncategorized] ~150-~150: Possible missing preposition found.
Context: ...ate existing state to Blac containers - Use alongside existing state management tem...
(AI_HYDRA_LEO_MISSING_TO)
CLAUDE.md
[uncategorized] ~38-~38: Loose punctuation mark.
Context: ...e container types: - Cubit<State>: Simple state container with direct `emi...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~60-~60: Loose punctuation mark.
Context: ...kages (/packages/) - @blac/core: Zero-dependency state management core -...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~65-~65: Loose punctuation mark.
Context: ...# Applications (/apps/) - demo/: Comprehensive usage examples showcasing...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~72-~72: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...evelopment Context ### Version Status Currently on v2.0.0-rc-3 (Release Candidate). Dev...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[duplication] ~85-~85: Possible typo: you repeated a word.
Context: ...ing Library** for component integration tests - Tests run in parallel across packages via Tur...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~94-~94: Possible missing comma found.
Context: ...eature Gaps See /TODO.md for planned features including: - Event transformation (deb...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
review.md
33-33: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
38-38: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
47-47: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
54-54: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
64-64: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
76-76: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
98-98: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
104-104: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
112-112: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
126-126: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
134-134: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
138-138: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🔇 Additional comments (4)
BLAC_IMPROVEMENTS_REVIEW.md (1)
44-60: Type safety improvements are well-documented
The transition fromanytounknownand addition of runtime guards is clearly described, and file lists are comprehensive.packages/blac-react/tests/useExternalBlocStore.test.tsx (1)
298-307: Await concurrent updates to avoid race-condition flakiness
Promise.allis started insideactbut never awaited, so the expect runs before all increments settle.
Make the test deterministic:- act(() => { - Promise.all([ - Promise.resolve().then(() => inst.increment()), - ... - ]); - }); + await act(async () => { + await Promise.all([ + Promise.resolve().then(() => inst.increment()), + ... + ]); + });Likely an incorrect or invalid review comment.
CLAUDE.md (1)
71-74: Version number is outdatedDocumentation says “Currently on v2.0.0-rc-3” while
package.jsonversions were bumped tov2.0.0-rc-4.
Update to avoid confusion.Likely an incorrect or invalid review comment.
packages/blac/src/BlocBase.ts (1)
25-25:crypto.randomUUID()may be undefined in Node ≤ 18 / old browsers.Relying on the global
cryptoobject breaks consumers on older runtimes. Provide a guarded fallback:-public uid = crypto.randomUUID(); +public uid = + typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function' + ? crypto.randomUUID() + : Math.random().toString(36).slice(2);
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/blac/src/Blac.ts (1)
174-204: StaticBlac.resetInstance()skips disposal – memory leak riskCalling
Blac.resetInstance()directly bypasses the instance-level cleanup of non-keepAlive blocs.
Those blocs stay alive forever (still referenced byuidRegistry), defeating the whole reset.Unify the logic:
-static resetInstance = (): void => { - instanceManager.resetInstance(); -}; +static resetInstance = (): void => { + Blac.instance.resetInstance(); // delegates to the instance method that performs disposal +};
🧹 Nitpick comments (2)
packages/blac/src/Blac.ts (2)
221-222: Stray debug log
this.log('dispatched bloc', bloc)looks like a leftover debug line and adds noise in prod builds.- this.log('dispatched bloc', bloc) + // this.log('disposeBloc executed', bloc); // enable if needed for debugging
594-605:validateConsumersschedules one timer per bloc – consolidate to avoid timer stormsWhen many blocs become orphaned simultaneously this creates N
setTimeoutcalls.
Consider batching with a single debounce/throttle timer or iterating synchronously if acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/blac/src/Blac.ts(17 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/blac/src/Blac.ts (2)
packages/blac/src/types.ts (5)
BlocHookDependencyArrayFn(60-62)BlocState(36-36)InferPropsFromGeneric(42-47)BlocConstructor(23-23)BlocBaseAbstract(15-17)packages/blac/src/BlocBase.ts (2)
BlocInstanceId(4-4)isIsolated(32-34)
| export interface GetBlocOptions<B extends BlocBase<unknown>> { | ||
| id?: string; | ||
| dependencySelector?: BlocHookDependencyArrayFn<BlocState<B>>; | ||
| selector?: BlocHookDependencyArrayFn<BlocState<B>>; | ||
| props?: InferPropsFromGeneric<B>; | ||
| onMount?: (bloc: B) => void; | ||
| instanceRef?: string; | ||
| throwIfNotFound?: boolean; | ||
| } |
There was a problem hiding this comment.
GetBlocOptions.id should accept number | undefined, not just string
id is consumed by helpers that expect the broader BlocInstanceId union.
Limiting it to string breaks callers passing numeric IDs and forces needless casts.
-export interface GetBlocOptions<B extends BlocBase<unknown>> {
- id?: string;
+export interface GetBlocOptions<B extends BlocBase<unknown>> {
+ id?: BlocInstanceId;📝 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.
| export interface GetBlocOptions<B extends BlocBase<unknown>> { | |
| id?: string; | |
| dependencySelector?: BlocHookDependencyArrayFn<BlocState<B>>; | |
| selector?: BlocHookDependencyArrayFn<BlocState<B>>; | |
| props?: InferPropsFromGeneric<B>; | |
| onMount?: (bloc: B) => void; | |
| instanceRef?: string; | |
| throwIfNotFound?: boolean; | |
| } | |
| export interface GetBlocOptions<B extends BlocBase<unknown>> { | |
| id?: BlocInstanceId; | |
| selector?: BlocHookDependencyArrayFn<BlocState<B>>; | |
| props?: InferPropsFromGeneric<B>; | |
| onMount?: (bloc: B) => void; | |
| instanceRef?: string; | |
| throwIfNotFound?: boolean; | |
| } |
🤖 Prompt for AI Agents
In packages/blac/src/Blac.ts around lines 20 to 27, the GetBlocOptions interface
defines the id property as an optional string, but it should accept both string
and number types to align with the broader BlocInstanceId union used by helpers.
Update the type of id to be number | string | undefined to allow numeric IDs
without requiring casts.
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
packages/blac/src/testing.ts (1)
10-125: Consider refactoring static-only class to module functions.The
BlocTestclass contains only static members, which is flagged by static analysis. Consider refactoring to module-level functions for better tree-shaking and cleaner API.-export class BlocTest { - private static originalInstance: Blac; +let originalInstance: Blac; - static setUp(): void { - this.originalInstance = Blac.instance; +export function setUp(): void { + originalInstance = Blac.instance; Blac.resetInstance(); Blac.enableLog = false; - } +} - static tearDown(): void { +export function tearDown(): void { Blac.resetInstance(); - } +} - static createBloc<T extends BlocBase<any>>( +export function createBloc<T extends BlocBase<any>>( BlocClass: new (...args: any[]) => T, ...args: any[] - ): T { +): T { const bloc = new BlocClass(...args); Blac.activateBloc(bloc); return bloc; - } +}packages/blac/src/Blac.ts (2)
20-27:GetBlocOptions.idshould acceptBlocInstanceIdtype.The
idproperty is limited tostringbut should accept the broaderBlocInstanceIdunion type (which includes numbers) to match the usage in helper methods.export interface GetBlocOptions<B extends BlocBase<unknown>> { - id?: string; + id?: BlocInstanceId; selector?: BlocHookDependencyArrayFn<BlocState<B>>; props?: InferPropsFromGeneric<B>; onMount?: (bloc: B) => void; instanceRef?: string; throwIfNotFound?: boolean; }
55-66: Keep-alive blocs need proper registration after transfer.When resetting the instance, keep-alive blocs are copied to
keepAliveBlocsanduidRegistrybut not registered inblocInstanceMaporisolatedBlocMap. This causes lookup methods to fail.resetInstance(): void { const oldInstance = SingletonBlacManager._instance; SingletonBlacManager._instance = new Blac({ __unsafe_ignore_singleton: true }); // Transfer any keep-alive blocs to the new instance if (oldInstance) { for (const bloc of oldInstance.keepAliveBlocs) { SingletonBlacManager._instance.keepAliveBlocs.add(bloc); SingletonBlacManager._instance.uidRegistry.set(bloc.uid, bloc); + // Properly register the bloc + if (bloc.isIsolated) { + SingletonBlacManager._instance.registerIsolatedBlocInstance(bloc); + } else { + SingletonBlacManager._instance.registerBlocInstance(bloc); + } } } }
🧹 Nitpick comments (31)
BLAC_CODE_REVIEW.md (17)
1-2: Align file naming with repository conventions
The file is named in uppercase snake_case (BLAC_CODE_REVIEW.md). Confirm whether documentation in this repo uses kebab-case, lowercase, or PascalCase for markdown files and rename accordingly.
3-6: Restructure Executive Summary for clarity
Instead of a dense paragraph, convert key takeaways into bullet points and explicitly call out the target release candidate (e.g.v2.0.0-rc-6) to align with PR context.
9-21: Clarify disposal fix implementation
The proposed “proper disposal queue or flag” is vague. Consider showing a guard-based approach, e.g.:class BlacObserver { - if (this.size === 0) { - this.bloc._dispose(); - } + if (this.size === 0 && !this.bloc._isDisposing) { + this.bloc._isDisposing = true; + disposalQueue.enqueue(this.bloc); + }This concretely prevents re-entrant disposal and centralizes cleanup.
45-52: Tighten generic constraints on events
ReplaceA extends objectwith a base event interface (e.g.interface BlocEvent { type: string; }). Then constrainA extends BlocEventto ensure predictable properties.
64-67: Prevent batch-state race conditions
Wrap the batching logic in a simple lock or mutex flag to serialize calls:if (this._batchLock) return; this._batchLock = true; try { /* ...batch update... */ } finally { this._batchLock = false; }
82-86: Improve error message context
Include bloc name, event type, and current state snapshot in every error to pinpoint failures. Example:Blac.error(`[${bloc.name}] add(${action.type}) failed`, { state: this.state });
91-101: Optimize dependency-tracking loop
The O(n) scan on every render can be expensive. Consider memoizing previous state keys or using a diff algorithm (e.g., only recalculating changed keys via shallow compare).
103-107: Cache proxies outside render
Moving proxy creation into a stableuseMemoor class field will avoid repeated lookups. Example:const proxy = useMemo(() => createProxy(store), [store]);
115-118: Add formal lifecycle-hook interfaces
Define and documentonCreate(),onActivate(),onDispose()on yourBlocBaseso implementers have a clear contract for side effects.
121-124: Provide testing utilities for blocs
Consumers currently must hand-roll mocks. Offer aBlocTestHarnessorMockBlocscaffold in your test utils to simplify setup/teardown.Would you like a draft of these helpers?
125-132: Expand debugging support with DevTools
Basicconsole.warnis insufficient. Consider integrating with Redux-DevTools protocol or adding a middleware API for time-travel and state inspection.
137-145: Remove leftover debugging code
The commented-outlogutility inuseBloc.tsxshould be deleted to clean up noise.
146-151: Clarify or remove orphan TODOs
Generic TODOs (e.g., eslint disables) lack context. Convert them into issues or expand with JIRA/GitHub Issue IDs so they don’t get forgotten.
168-175: Prioritize and contextualize immediate actions
Group high-priority fixes by owner or severity and link to issues. This will help the team focus on “Fix Memory Leaks” before secondary tasks.
176-182: Group short-term improvements by theme
Consider categorizing “Performance”, “UX/DevTools”, and “API” under sub-headings to improve readability and tracking.
184-189: Refine long-term enhancements with feasibility notes
Annotate each item (SSR, persistence, etc.) with rough effort estimates or milestone targets to guide planning.
191-199: Convert positive aspects to bullet list
A concise bullet format will make strengths stand out and improve scannability.CRITICAL_FIXES_SUMMARY.md (2)
3-3: Consider formatting date consistently.The static analysis tool suggests adding a comma after the year in the date format for better readability.
-**Date:** June 23, 2025 +**Date:** June 23, 2025,
136-139: Add language specification to code block.The test results code block should specify a language for proper syntax highlighting.
-**Test Results:** -``` +**Test Results:** +```bash ✓ tests/MemoryManagement.test.ts (8 tests) 3ms ✓ All core package tests passing</blockquote></details> <details> <summary>REVIEW_25-06-23.md (1)</summary><blockquote> `2-2`: **Consider formatting date consistently.** Similar to the other document, consider adding a comma after the year for consistency with style guidelines. ```diff -**Date:** June 23, 2025 +**Date:** June 23, 2025,BLAC_CRITICAL_FIXES_LOG.md (4)
83-83: Fix markdown heading punctuationRemove the trailing colon from the heading to follow markdown best practices.
-### Key Testing Features: +### Key Testing Features
124-124: Fix markdown heading punctuationRemove the trailing colon from the heading.
-### New Documentation Created: +### New Documentation Created
130-130: Fix markdown heading punctuationRemove the trailing colon from the heading.
-### Testing Guide Covers: +### Testing Guide Covers
141-141: Use proper heading format instead of emphasisConvert the emphasized text to a proper heading for better document structure.
-**STATUS: MISSION ACCOMPLISHED! 🌟** +### STATUS: MISSION ACCOMPLISHED! 🌟packages/blac/src/Bloc.ts (1)
149-158: Consider making error handling behavior configurableThe TODO comment suggests implementing an error boundary pattern. Consider adding a configuration option to the Bloc class to control error handling behavior (log only, emit error state, or re-throw).
Would you like me to implement a configurable error handling mechanism that allows users to choose between different error handling strategies?
packages/blac/examples/testing-example.test.ts (2)
42-47: Consider using fake timers for more reliable tests.Using real
setTimeoutin tests can lead to flaky tests due to timing variations. Consider using Vitest's fake timers for more deterministic testing.async incrementAsync() { this.setLoading(true); - await new Promise(resolve => setTimeout(resolve, 100)); + await new Promise(resolve => setTimeout(resolve, 100)); this.increment(); this.setLoading(false); }In your tests, use:
import { vi } from 'vitest'; // In test setup vi.useFakeTimers(); // In test await vi.advanceTimersByTimeAsync(100); // In test cleanup vi.useRealTimers();
327-341: Verify that errors are actually logged.The test expects errors to be caught internally but doesn't verify that they were logged. Consider mocking console methods or using a custom logger to verify error handling.
it('should test error scenarios with mocked blocs', async () => { const mockBloc = new MockBloc<UserState>({ id: null, name: '', email: '' }); + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); // Mock an error scenario - the error should be caught and logged, not thrown mockBloc.mockEventHandler(LoadUserEvent, async (event, emit) => { throw new Error('Network error'); }); // The add method should complete successfully (error is caught internally) // but no state change should occur await mockBloc.add(new LoadUserEvent('user-123')); // Verify the state didn't change due to the error expect(mockBloc.state).toEqual({ id: null, name: '', email: '' }); + + // Verify error was logged + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Network error') + ); + + consoleErrorSpy.mockRestore(); });packages/blac/src/testing.ts (1)
109-117: JSON.stringify comparison has limitations.Using
JSON.stringifyfor deep equality checks won't handle circular references, functions, undefined values, symbols, or Date objects correctly. Consider using a proper deep equality library.-// Use JSON comparison for deep equality -if (JSON.stringify(expected) !== JSON.stringify(received)) { +// Use a proper deep equality check +if (!isDeepEqual(expected, received)) { reject(new Error( `State mismatch at index ${i}. Expected: ${JSON.stringify(expected)}, ` + `Received: ${JSON.stringify(received)}` )); return; }You could use a utility like:
function isDeepEqual(a: any, b: any): boolean { // Use a library like lodash.isEqual or implement a proper deep comparison return JSON.stringify(a) === JSON.stringify(b); // Fallback for now }packages/blac/src/BlocBase.ts (3)
25-25: Consider makinguidproperty readonly to prevent external modification.The public
uidproperty could be accidentally modified by external code, potentially breaking internal tracking.- public uid = generateUUID(); + public readonly uid = generateUUID();
135-154: Optimize consumer validation to avoid unnecessary iterations.The
_validateConsumersmethod iterates through all consumer references on every call, which could be expensive for blocs with many consumers. Consider implementing a lazy validation approach or debouncing.+ private _needsValidation = false; + private _validationScheduled = false; + + private _scheduleValidation = () => { + if (!this._validationScheduled) { + this._validationScheduled = true; + queueMicrotask(() => { + this._validationScheduled = false; + if (this._needsValidation) { + this._validateConsumers(); + this._needsValidation = false; + } + }); + } + };Then call
this._scheduleValidation()instead of immediate validation where needed.
308-322: Consider adding disposal timeout for better resource management.The disposal scheduling logic is sound, but consider adding a timeout mechanism to prevent memory leaks if disposal handlers fail or take too long.
+ private _disposalTimeoutId?: NodeJS.Timeout; + private _scheduleDisposal() { // Prevent multiple disposal attempts if (this._disposalState !== 'active') { return; } + // Clear any existing disposal timeout + if (this._disposalTimeoutId) { + clearTimeout(this._disposalTimeoutId); + } + + // Schedule disposal with timeout fallback + this._disposalTimeoutId = setTimeout(() => { // Double-check conditions before disposal if (this._consumers.size === 0 && !this._keepAlive) { if (this._disposalHandler) { this._disposalHandler(this as any); } else { this._dispose(); } } + }, 0); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.cursor/rules/base.mdc(1 hunks)AGENTS.md(1 hunks)BLAC_CODE_REVIEW.md(1 hunks)BLAC_CRITICAL_FIXES_LOG.md(1 hunks)CRITICAL_FIXES_SUMMARY.md(1 hunks)REVIEW_25-06-23.md(1 hunks)packages/blac-react/package.json(2 hunks)packages/blac-react/src/useBloc.tsx(3 hunks)packages/blac-react/src/useExternalBlocStore.ts(1 hunks)packages/blac-react/tests/useExternalBlocStore.edgeCases.test.tsx(1 hunks)packages/blac-react/tests/useExternalBlocStore.test.tsx(1 hunks)packages/blac/README.md(4 hunks)packages/blac/docs/testing.md(1 hunks)packages/blac/examples/testing-example.test.ts(1 hunks)packages/blac/package.json(2 hunks)packages/blac/src/Blac.ts(16 hunks)packages/blac/src/BlacObserver.ts(6 hunks)packages/blac/src/Bloc.ts(4 hunks)packages/blac/src/BlocBase.ts(9 hunks)packages/blac/src/index.ts(1 hunks)packages/blac/src/testing.ts(1 hunks)packages/blac/src/types.ts(1 hunks)packages/blac/src/utils/uuid.ts(1 hunks)packages/blac/tests/Blac.test.ts(12 hunks)packages/blac/tests/Bloc.test.ts(1 hunks)packages/blac/tests/BlocBase.test.ts(3 hunks)packages/blac/tests/MemoryManagement.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/blac/src/utils/uuid.ts
- AGENTS.md
- .cursor/rules/base.mdc
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/blac/package.json
- packages/blac-react/package.json
- packages/blac/src/index.ts
- packages/blac/tests/Blac.test.ts
- packages/blac/tests/BlocBase.test.ts
- packages/blac/src/BlacObserver.ts
- packages/blac-react/src/useExternalBlocStore.ts
🧰 Additional context used
🪛 LanguageTool
REVIEW_25-06-23.md
[style] ~2-~2: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...c/core & @blac/react Date: June 23, 2025 Reviewer: Claude Code **Scope:*...
(MISSING_COMMA_AFTER_YEAR)
[uncategorized] ~76-~76: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Standardize error handling strategy ## High Priority Issues ### 4. Performance Concerns...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~106-~106: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...rn with configurable error handling ## Medium Priority Issues ### 6. **API Design Inconsisten...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~136-~136: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...st isolation and cleanup mechanisms ## Low Priority Issues ### 8. **Documentation & Develo...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
BLAC_CODE_REVIEW.md
[uncategorized] ~105-~105: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...:94-147` Problem: Proxies are cached but the cache lookup happens on every rende...
(COMMA_COMPOUND_SENTENCE_2)
[style] ~204-~204: Consider using a different verb for a more formal wording.
Context: ...0.0-rc-5) should focus on stability and fixing the critical issues before adding new f...
(FIX_RESOLVE)
CRITICAL_FIXES_SUMMARY.md
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...ace Condition Fixes Date: June 23, 2025 Status: ✅ COMPLETED **Test Stat...
(MISSING_COMMA_AFTER_YEAR)
🪛 markdownlint-cli2 (0.17.2)
REVIEW_25-06-23.md
10-10: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
196-196: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
204-204: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
212-212: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
BLAC_CRITICAL_FIXES_LOG.md
83-83: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
124-124: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
130-130: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
141-141: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
CRITICAL_FIXES_SUMMARY.md
136-136: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 Biome (1.9.4)
packages/blac/src/testing.ts
[error] 10-125: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 17-17: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 133-135: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (28)
CRITICAL_FIXES_SUMMARY.md (1)
1-162: Excellent comprehensive documentation of critical fixes.This document provides clear, well-structured documentation of important technical improvements. The fixes address real production concerns around memory management and race conditions. The code examples are helpful and demonstrate proper implementation patterns.
Minor suggestions for improvement:
- Consider adding performance benchmarks before/after the fixes
- Include migration notes if any API changes affect consumers
packages/blac/tests/Bloc.test.ts (3)
196-203: Enhanced warning message validation improves test coverage.The updated test assertions properly validate the enhanced warning message format with additional context. This ensures the improved diagnostics work as expected.
207-207: Good variable naming improvement.Changing
errorActiontoerrorThrowingActionmakes the test more descriptive and clear about the intent.
213-226: Comprehensive error context validation.The enhanced error message validation ensures that the improved error handling provides rich diagnostic information including bloc identity, event details, and timing information. This is excellent for debugging production issues.
packages/blac/src/types.ts (4)
57-63: Well-designed base event interface.The
BlocEventinterface provides a solid foundation for the event-driven architecture with optionaltypeandtimestampproperties that support debugging and event tracking.
65-68: Appropriate event constraint for type safety.The
BlocEventConstrainttype properly combines theBlocEventinterface with object constraint, ensuring events are both properly typed and have the necessary object properties for the event system.
70-76: Comprehensive error boundary interface.The
BlocErrorBoundaryinterface provides excellent support for error handling with detailed context and configurable error propagation. TheonErrormethod receives all necessary context for proper error handling and debugging.
84-84: Good syntax cleanup.Removing the trailing comma improves code consistency and follows TypeScript best practices.
packages/blac/docs/testing.md (3)
1-529: Outstanding comprehensive testing documentation.This documentation is exceptionally well-written and comprehensive. It covers all aspects of testing with Blac, including:
- Clear setup and usage examples
- Multiple testing utilities with detailed explanations
- Best practices and real-world scenarios
- Integration with popular testing frameworks
- Comprehensive API reference
The structure makes it easy to navigate, and the examples are practical and immediately usable. This will significantly improve the developer experience for testing Blac applications.
125-134: Excellent error handling documentation.The section on error handling and timeout behavior is particularly valuable, showing developers how to handle edge cases properly in their tests.
280-296: Valuable memory leak detection guidance.The MemoryLeakDetector documentation with detailed reporting examples will help developers identify and fix memory issues proactively. This is especially important for long-running applications.
REVIEW_25-06-23.md (3)
1-228: Comprehensive and well-structured technical review.This technical review document is excellent. It provides:
- Clear prioritization of issues from critical to low priority
- Specific file locations and code examples
- Actionable recommendations with timelines
- Balanced assessment including architectural strengths
- Professional grading system across multiple dimensions
The issues identified align well with the fixes described in
CRITICAL_FIXES_SUMMARY.md, showing good consistency between problem identification and solution implementation.
16-31: Good alignment between identified problems and implemented fixes.The memory management issues identified here (WeakSet not being used effectively, race conditions in disposal) are exactly what's addressed in the
CRITICAL_FIXES_SUMMARY.md. This shows good problem identification and solution implementation.
57-74: React integration issues properly identified.The proxy-based dependency tracking issues and inconsistent error handling problems are well-documented. These align with the React integration improvements mentioned in the AI summary.
packages/blac-react/tests/useExternalBlocStore.test.tsx (1)
1-451: Excellent test coverage!This test suite comprehensively covers the
useExternalBlocStorehook functionality including basic operations, subscription management, dependency tracking, props handling, edge cases, memory management, and SSR support. The tests are well-structured with proper setup/teardown and good use of mocking.packages/blac/tests/MemoryManagement.test.ts (1)
1-170: Well-designed memory management tests!This test suite effectively validates the critical memory management improvements including WeakRef consumer tracking, disposal race condition prevention, and memory statistics. The acknowledgment of garbage collection limitations in the test environment (lines 61-70) shows good understanding of testing constraints.
packages/blac/src/Bloc.ts (1)
27-169: Excellent implementation of sequential event processing!The event queue mechanism effectively prevents race conditions by ensuring events are processed sequentially. The enhanced error handling with rich context information will greatly help with debugging. The code is well-documented and follows best practices.
packages/blac-react/tests/useExternalBlocStore.edgeCases.test.tsx (1)
1-412: Outstanding edge case test coverage!This test suite thoroughly exercises edge cases including complex state handling (Maps, Sets, Symbols), error scenarios, dependency tracking edge cases, concurrency issues, and memory/performance considerations. The tests are well-organized and use proper mocking techniques for error scenarios. This level of testing will help ensure robustness in production.
packages/blac/src/testing.ts (1)
133-135: Remove unnecessary constructor.This constructor only calls super and can be omitted.
packages/blac/README.md (1)
28-69: Excellent testing documentation!The new testing section provides clear examples and covers the key testing utilities. The examples are practical and demonstrate real-world usage patterns.
packages/blac-react/src/useBloc.tsx (1)
111-115: Verify setTimeout(0) doesn't cause timing issues.Using
setTimeout(0)to defer dependency tracking until after render is a valid approach, but it might miss dependencies if the component unmounts immediately or if state changes happen synchronously after render.Consider if this edge case needs handling or document why it's safe in your use case.
packages/blac/src/Blac.ts (1)
568-581: Avoid concurrent modification when disposing keep-alive blocs.Iterating over
keepAliveBlocswhilebloc._dispose()removes items from the same set can cause concurrent modification issues. The current code creates a temporary array which is good, but could be clearer.The implementation correctly avoids concurrent modification by collecting blocs to dispose in a separate array first. Consider adding a comment to make this pattern clearer:
disposeKeepAliveBlocs = <B extends BlocConstructor<any>>( blocClass?: B ): void => { + // Collect blocs first to avoid concurrent modification const toDispose: BlocBase<unknown>[] = [];packages/blac/src/BlocBase.ts (6)
167-172: Excellent type-safe static property access.The defensive type checking and fallback defaults prevent runtime errors from malformed static properties.
209-231: Well-designed atomic disposal state management.The three-state disposal logic effectively prevents race conditions and ensures idempotent disposal operations. The state transitions are clear and well-protected.
340-370: Robust input validation with helpful debugging.The validation logic properly handles edge cases and provides clear warning messages. The batching vs. normal flow separation is well-implemented.
376-404: Excellent batching implementation with race condition protection.The batching logic correctly:
- Prevents nested batching with
_batchingLock- Accumulates updates during batch execution
- Notifies observers only once with final state
- Ensures cleanup in finally block
This is a well-designed pattern for performance optimization.
2-2: Verify UUID utility import path and implementation.Ensure the UUID utility exists and provides secure, collision-resistant identifiers.
#!/bin/bash # Verify the UUID utility exists and examine its implementation fd -t f uuid.ts packages/blac/src/utils/ cat packages/blac/src/utils/uuid.ts
253-268: Validate consumer reference storage strategy.The WeakRef approach is good for memory management, but consider whether storing consumer references is necessary if they're only used for validation.
#!/bin/bash # Search for usage patterns of consumerRef parameter and _consumerRefs map rg -A 3 -B 3 "_consumerRefs\." packages/blac/src/ rg -A 3 -B 3 "consumerRef" packages/blac/src/
BLAC_CODE_REVIEW.md
Outdated
| ### 4. Error Handling and Developer Experience | ||
| #### Issue: Silent Failures in Event Handlers | ||
| **Location**: `Bloc.ts:83-95` | ||
| ```typescript | ||
| } catch (error) { | ||
| Blac.error(...); | ||
| // Error is logged but not propagated | ||
| } | ||
| ``` | ||
| **Problem**: Errors in event handlers are swallowed, making debugging difficult. | ||
| **Fix**: Add an error boundary mechanism or optional error propagation. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Propagate errors instead of swallowing silently
Catching without rethrowing or emitting makes debugging painful. After logging, rethrow or surface via an onError callback so consumers can handle failures explicitly.
🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 69 to 81, the error handling in event
handlers at Bloc.ts lines 83-95 currently logs errors but does not propagate
them, causing silent failures. To fix this, after logging the error with
Blac.error, either rethrow the error or invoke an onError callback if provided,
so that consumers of the event handlers can explicitly handle or respond to
these errors.
BLAC_CODE_REVIEW.md
Outdated
| ## Conclusion | ||
|
|
||
| Blac shows promise as a state management solution with its clean API and TypeScript-first approach. However, the critical issues around memory management, type safety, and error handling must be addressed before it's ready for production use. The library would benefit from more robust error handling, better debugging tools, and implementation of the documented but missing features. | ||
|
|
There was a problem hiding this comment.
Update release candidate reference
The conclusion mentions v2.0.0-rc-5 but the PR is targeting -rc-6. Please synchronize the version across the report.
🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 200 to 203, the conclusion references
version v2.0.0-rc-5, but the PR targets -rc-6. Update the version mention in the
conclusion to v2.0.0-rc-6 to keep the document consistent with the current
release candidate.
BLAC_CODE_REVIEW.md
Outdated
| #### Issue: Inconsistent Export Strategy | ||
| **Location**: `package.json` files | ||
| **Problem**: Mix of `src/index.ts` and `dist/` exports can cause issues. | ||
| **Fix**: Standardize on one approach. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardize package export strategy
Mixing src/index.ts and dist/ entry points can confuse consumers. Pick one canonical entry in all package.json files for consistency.
🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 161 to 165, the package.json files
inconsistently export using both src/index.ts and dist/ paths. To fix this,
choose either the source (src/index.ts) or the built distribution (dist/) as the
single canonical export entry point and update all package.json files to use
this consistently, ensuring all exports align across the project.
BLAC_CODE_REVIEW.md
Outdated
|
|
||
| #### Issue: Inconsistent Naming | ||
| - `_dispose()` vs `dispose()` vs `onDispose()` | ||
| - `emit()` in Cubit vs `add()` in Bloc | ||
| - `patch()` only available in Cubit, not Bloc | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardize API naming across Bloc/Cubit
Align methods like emit() vs add(), and unify disposal hooks names (dispose() vs _dispose() vs onDispose()). A consistent naming scheme reduces cognitive load.
🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 109 to 114, the naming conventions for
methods across Bloc and Cubit are inconsistent, such as `emit()` vs `add()` and
disposal methods `_dispose()`, `dispose()`, and `onDispose()`. Standardize these
method names by choosing a single naming pattern for emission (either `emit()`
or `add()`) and unify disposal method names to one consistent term across both
Bloc and Cubit to reduce confusion and improve API consistency.
BLAC_CODE_REVIEW.md
Outdated
| #### Issue: Isolated Bloc Memory Management | ||
| **Location**: `Blac.ts:288-307` | ||
| **Problem**: Isolated blocs are stored in both `isolatedBlocMap` and `isolatedBlocIndex` but cleanup may miss one of them. | ||
| **Fix**: Ensure both data structures are always synchronized or use a single source of truth. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate isolated-bloc cleanup data structures
Storing isolated blocs in both a map and an index risks out-of-sync removals. Use a single Map<BlocID, Bloc> or wrap both in a helper class that ensures atomic updates.
🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 30 to 34, the isolated blocs are stored in
both isolatedBlocMap and isolatedBlocIndex, which risks inconsistent cleanup.
Refactor the code to use a single Map<BlocID, Bloc> for storing isolated blocs
or encapsulate both data structures within a helper class that manages updates
atomically, ensuring both remain synchronized during additions and removals.
BLAC_CODE_REVIEW.md
Outdated
| #### Issue: Async Event Handling Without Queue | ||
| **Location**: `Bloc.ts:60-108` | ||
| ```typescript | ||
| public add = async (action: A): Promise<void> => { | ||
| // No queuing mechanism for concurrent events | ||
| ``` | ||
| **Problem**: Multiple async events can be processed simultaneously, potentially causing state inconsistencies. | ||
| **Fix**: Implement an event queue or use the documented concurrent processing flag. | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Introduce event queue for async handlers
Without queuing, concurrent add calls may interleave. Implement a FIFO queue, e.g.:
private _queue: A[] = [];
public add = async (action: A) => {
this._queue.push(action);
if (this._queue.length === 1) {
while (this._queue.length) {
await this._process(this._queue[0]);
this._queue.shift();
}
}
};🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 55 to 63, the add method in Bloc.ts lacks a
queuing mechanism, allowing multiple async events to be processed concurrently
and causing potential state inconsistencies. Fix this by implementing a FIFO
event queue: add a private queue array to store incoming actions, push each new
action into this queue, and process them sequentially by awaiting the processing
of the first action in the queue before shifting it out and moving to the next,
ensuring only one event is processed at a time.
BLAC_CODE_REVIEW.md
Outdated
| #### Issue: WeakSet Consumer Tracking Not Used | ||
| **Location**: `BlocBase.ts:189` | ||
| ```typescript | ||
| private _consumerRefs = new WeakSet<object>(); | ||
| ``` | ||
| **Problem**: Consumer refs are added but never checked, making it impossible to validate if consumers are still alive. | ||
| **Fix**: Implement periodic validation or remove if not needed. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address unused WeakSet consumer tracking
The _consumerRefs WeakSet is never read. Either remove it to reduce complexity or implement a periodic cleanup/validation routine. If you plan to keep it, show how and where you’ll check has() on registered consumers.
🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 22 to 29, the WeakSet _consumerRefs is
declared but never used, which adds unnecessary complexity. To fix this, either
remove the _consumerRefs declaration entirely if consumer tracking is not
needed, or implement logic that periodically checks if consumers are still alive
by calling _consumerRefs.has() on registered objects and cleaning up
accordingly. Ensure that if kept, the usage of _consumerRefs is clearly
demonstrated in the code.
BLAC_CODE_REVIEW.md
Outdated
|
|
||
| #### Issue: Missing JSDoc in Key Methods | ||
| - `batch()` method lacks documentation | ||
| - `_pushState()` internal workings not documented | ||
| - No examples in code comments | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JSDoc for key internal methods
Document batch(), _pushState(), and any public APIs so consumers know correct usage patterns.
🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md around lines 153 to 158, the batch() and _pushState()
methods lack JSDoc comments, and there are no usage examples in the code
comments. Add comprehensive JSDoc comments for these methods describing their
purpose, parameters, return values, and any side effects. Also, include example
usage snippets in the comments to help consumers understand how to use these
APIs correctly.
BLAC_CODE_REVIEW.md
Outdated
| ### 2. Type Safety Issues | ||
|
|
||
| #### Issue: Unsafe Type Assertions | ||
| **Location**: `useBloc.tsx:169`, `useExternalBlocStore.ts:182` | ||
| ```typescript | ||
| return [returnState as BlocState<InstanceType<B>>, returnClass as InstanceType<B>]; | ||
| ``` | ||
| **Problem**: Unsafe type assertions that could hide runtime errors. | ||
| **Fix**: Add proper type guards or ensure types are correctly inferred. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety of returned tuple
Avoid as assertions in hooks; introduce a runtime type guard or leverage generics to infer BlocState<A>. For example:
function isState<S>(val: unknown): val is S { /* ... */ }
return isState(returnState) ? [returnState, returnClass] : throw new TypeError();🤖 Prompt for AI Agents
In BLAC_CODE_REVIEW.md lines 35 to 44, the code uses unsafe type assertions in
the return statement of hooks at useBloc.tsx line 169 and
useExternalBlocStore.ts line 182, which can mask runtime errors. To fix this,
replace the 'as' assertions with proper runtime type guards that verify the type
of returnState before returning it, or adjust the generics so the types are
correctly inferred without assertions. Implement a type guard function that
checks the type and throw a TypeError if the check fails, ensuring type safety
at runtime.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
apps/demo/App.tsx (1)
150-150: Type mismatch in setShow callback.The
setShowprop is typed to accept a boolean parameter, but you're passing a function that ignores it. This should be aligned for consistency.apps/demo/components/StaticPropsDemo.tsx (5)
9-9: Replaceanytype with a proper interface.The
datafield should have a proper type definition instead ofanyto maintain type safety.
44-44: Avoid accessing private properties via type casting.Accessing
_idthrough(this as any)breaks encapsulation. Consider exposing a public getter method or property.
77-83: Add error handling for external avatar service and validate date construction.The external avatar service could fail and should have error handling. Also, using
instanceNumberas a day could create invalid dates.
243-243: Use the library's ID generation function instead of hardcoding.The manual construction of
generatedIdmay become out of sync with the actual implementation. Consider importing and using the actual ID generation function.
173-173: Avoid accessing private _id property.Similar to line 44, this accesses the private
_idfield breaking encapsulation. Use a public getter instead.- <div>Instance ID: {(cubit as any)._id}</div> + <div>Instance ID: {cubit.id}</div>
🧹 Nitpick comments (6)
apps/demo/App.tsx (4)
12-12: Remove commented out code.This commented import should be removed as it's not being used and the comment already explains the reason.
-// import { Card, CardContent, CardDescription, CardHeader, CardTitle } from './components/ui/Card'; // Removing Card components for simpler styling
54-62: Remove unused props from interface.The
titleColorandtitleBgprops are defined but never used (as noted in line 63). Remove them from the interface to avoid confusion.const DemoCard: React.FC<{ title: string; description: string; children: React.ReactNode; - titleColor?: string; - titleBg?: string; show: boolean; setShow: (show: boolean) => void; }> = ({ title, description, children, show, setShow }) => {
104-128: Simplify repetitive state update pattern.The setShow handlers follow a repetitive pattern. Consider creating a helper function to reduce duplication.
function App() { const showDefault = false; const [show, setShow] = useState({ basic: showDefault, // ... other properties }); + + const toggleDemo = (key: keyof typeof show) => { + setShow(prev => ({ ...prev, [key]: !prev[key] })); + }; // Then in each DemoCard: - setShow={() => setShow({ ...show, basic: !show.basic })} + setShow={() => toggleDemo('basic')}Also applies to: 150-151, 159-161
388-394: Replace placeholder links with actual URLs or remove them.The footer contains placeholder "#" links that don't lead anywhere. Either implement proper links or remove them if not needed.
- <a href="#" style={LINK_STYLE}> + <a href="https://your-docs-url.com" style={LINK_STYLE}> Documentation </a>{' '} |{' '} - <a href="#" style={LINK_STYLE}> + <a href="https://github.com/your-repo" style={LINK_STYLE}> GitHub </a>apps/demo/components/CustomPluginDemo.tsx (1)
185-187: Use specific types instead ofany[].Define proper interfaces for the plugin data instead of using
any[].+interface AnalyticsEvent { + timestamp: number; + event: string; + bloc: string; + data?: any; +} + +interface PerformanceMetric { + bloc: string; + updates: number; + avgTime: string | number; +} + +interface ValidationError { + bloc: string; + error: string; +} - const [analyticsEvents, setAnalyticsEvents] = useState<any[]>([]); - const [performanceMetrics, setPerformanceMetrics] = useState<any[]>([]); - const [validationErrors, setValidationErrors] = useState<any[]>([]); + const [analyticsEvents, setAnalyticsEvents] = useState<AnalyticsEvent[]>([]); + const [performanceMetrics, setPerformanceMetrics] = useState<PerformanceMetric[]>([]); + const [validationErrors, setValidationErrors] = useState<ValidationError[]>([]);apps/demo/components/AsyncOperationsDemo.tsx (1)
9-9: Replaceanywith a specific data interface.Define a proper interface for the API response data instead of using
any.+interface ApiResponseData { + id: string; + timestamp: string; + message: string; + value: number; +} + interface ApiState { - data: any | null; + data: ApiResponseData | null; loading: boolean; error: string | null; successCount: number; errorCount: number; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/demo/App.tsx(1 hunks)apps/demo/LcarsHeader.tsx(1 hunks)apps/demo/components/AdvancedSelectorsDemo.tsx(1 hunks)apps/demo/components/AsyncOperationsDemo.tsx(1 hunks)apps/demo/components/BlocWithReducerDemo.tsx(1 hunks)apps/demo/components/CustomPluginDemo.tsx(1 hunks)apps/demo/components/ExternalStoreDemo.tsx(1 hunks)apps/demo/components/RerenderLoggingDemo.tsx(1 hunks)apps/demo/components/StaticPropsDemo.tsx(1 hunks)apps/demo/components/StreamApiDemo.tsx(1 hunks)apps/demo/components/TestingUtilitiesDemo.tsx(1 hunks)apps/demo/components/ui/Input.tsx(1 hunks)apps/demo/components/ui/Label.tsx(1 hunks)apps/demo/main.tsx(1 hunks)apps/demo/package.json(1 hunks)apps/demo/tsconfig.json(1 hunks)apps/docs/.eslintignore(0 hunks)apps/docs/eslint.config.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- apps/docs/.eslintignore
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/demo/components/ui/Input.tsx
- apps/demo/tsconfig.json
- apps/demo/components/ui/Label.tsx
- apps/demo/components/RerenderLoggingDemo.tsx
- apps/demo/main.tsx
- apps/demo/package.json
- apps/demo/LcarsHeader.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: unit tests are written using vitest, and tests should be run with the command 'pnpm run test <...>' ...
Learnt from: CR
PR: jsnanigans/blac#0
File: .cursor/rules/base.mdc:0-0
Timestamp: 2025-06-25T11:19:15.871Z
Learning: Unit tests are written using vitest, and tests should be run with the command 'pnpm run test <...>' from within the relevant package or apps directory.
Applied to files:
apps/demo/components/TestingUtilitiesDemo.tsx
🧬 Code Graph Analysis (2)
apps/demo/components/TestingUtilitiesDemo.tsx (2)
packages/blac/src/testing.ts (1)
BlocTest(10-127)apps/demo/components/ui/Button.tsx (1)
Button(125-125)
apps/demo/App.tsx (4)
packages/blac/src/Blac.ts (1)
Blac(102-800)apps/demo/lib/styles.ts (10)
SECTION_STYLE(41-48)FONT_FAMILY_SANS(2-3)COLOR_PRIMARY_ACCENT(10-10)COLOR_SECONDARY_ACCENT(12-12)COLOR_TEXT_SECONDARY(9-9)DEMO_COMPONENT_CONTAINER_STYLE(148-154)APP_CONTAINER_STYLE(21-28)HEADER_STYLE(30-39)FOOTER_STYLE(103-111)LINK_STYLE(92-96)apps/demo/components/ui/Button.tsx (1)
Button(125-125)apps/demo/components/RerenderLoggingDemo.tsx (1)
RerenderLoggingDemo(13-307)
🔇 Additional comments (21)
apps/docs/eslint.config.mjs (5)
1-3: LGTM!The imports are correctly structured for a modern ESLint flat configuration with proper TypeScript and React hooks support.
7-19: Comprehensive ignore patterns are well-configured.The ignore list appropriately covers build outputs, caches, and temporary directories commonly found in documentation projects, including VitePress-specific paths.
21-22: Proper configuration extension pattern.The configuration correctly extends both JavaScript and TypeScript recommended rules using the modern flat config syntax.
24-24: React hooks plugin properly registered.The plugin registration follows the correct flat config pattern for enabling React hooks rules.
25-32: Well-configured custom rules for a docs project.The rule configuration is appropriate:
- React hooks exhaustive deps as warning balances code quality with build stability
- TypeScript unused vars rule properly handles underscore-prefixed variables
- Disabling
no-undefis correct when TypeScript handles type checkingapps/demo/components/StreamApiDemo.tsx (3)
6-30: LGTM! Clean Cubit implementationThe
StreamDemoCubitclass is well-structured with proper state immutability patterns using spread operators.
41-90: Excellent subscription management with proper cleanupThe subscription patterns effectively demonstrate various use cases, and all subscriptions are properly cleaned up to prevent memory leaks.
92-194: Well-structured UI with informative documentationThe component provides a clear demonstration of the Stream API with helpful visual feedback and documentation.
apps/demo/components/ExternalStoreDemo.tsx (3)
7-15: Clean external Cubit implementationThe
ExternalCounterCubitclass provides a simple and effective example of an external store.
17-23: Good demonstration of external store patternCreating the instance outside React effectively shows how BlaC can manage global state accessible from anywhere.
44-50: Proper manual subscription with cleanupThe manual subscription correctly demonstrates direct subscription API usage with appropriate cleanup.
apps/demo/components/BlocWithReducerDemo.tsx (4)
15-45: Well-designed event classes with immutable propertiesThe event classes follow a consistent pattern with readonly properties, ensuring event immutability.
65-96: Correct implementation of add item logicThe handler properly checks for existing items and maintains state immutability.
108-123: Smart quantity update with automatic removalGood design decision to automatically remove items when quantity reaches zero or below.
177-196: Efficient computed getters for derived valuesThe getters provide clean access to calculated values without duplicating state.
apps/demo/components/AdvancedSelectorsDemo.tsx (2)
35-158: Well-implemented complex state managementThe
AppStateCubitcorrectly handles deeply nested state updates with proper immutability patterns and provides useful computed getters.
161-307: Excellent demonstration of advanced selector patternsThe child components effectively showcase various selector strategies including simple paths, computed values, and conditional dependencies, with helpful render count tracking.
apps/demo/components/TestingUtilitiesDemo.tsx (2)
6-34: Well-designed test cubitsThe test cubits provide good examples for testing different aspects of state management, including history tracking.
228-271: Good performance benchmarking implementationThe benchmarks provide useful metrics for state updates and subscription overhead with proper cleanup.
apps/demo/components/AsyncOperationsDemo.tsx (2)
135-258: Well-implemented async search with cancellation!The SearchBloc demonstrates excellent patterns for handling async operations:
- Proper cancellation of previous searches
- Clean abort signal handling
- Good error differentiation between cancelled and failed searches
260-467: Clean implementation of the demo UI.The component effectively demonstrates async operations with:
- Clear visual feedback for loading, success, and error states
- Good separation between different async patterns (simple fetch vs cancellable search)
- Comprehensive error handling and user controls
| const entry = { | ||
| timestamp: Date.now(), | ||
| event, | ||
| bloc: bloc._name || 'Unknown', |
There was a problem hiding this comment.
Avoid accessing private _name property.
The plugins are accessing the private _name property of blocs. This breaks encapsulation and could break if the internal implementation changes. Consider using a public API to get the bloc name.
- bloc: bloc._name || 'Unknown',
+ bloc: bloc.name || bloc.constructor.name || 'Unknown',The Bloc/Cubit base classes should expose a public name getter if plugins need this information.
Also applies to: 70-70, 119-119
🤖 Prompt for AI Agents
In apps/demo/components/CustomPluginDemo.tsx at lines 36, 70, and 119, the code
accesses the private property _name of bloc instances, which breaks
encapsulation. To fix this, update the Bloc/Cubit base classes to include a
public getter named name that returns the bloc's name, then replace all
occurrences of bloc._name with bloc.name to use this public API instead of the
private property.
| if (global.gc) { | ||
| global.gc(); | ||
| } |
There was a problem hiding this comment.
Potential issue with global.gc
global.gc is not guaranteed to be available in all JavaScript environments. Consider checking for its existence more safely or documenting that this test requires running with --expose-gc flag.
Apply this diff to handle the potential undefined global:
- if (global.gc) {
- global.gc();
- }
+ if (typeof global !== 'undefined' && global.gc) {
+ global.gc();
+ }📝 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 (global.gc) { | |
| global.gc(); | |
| } | |
| if (typeof global !== 'undefined' && global.gc) { | |
| global.gc(); | |
| } |
🤖 Prompt for AI Agents
In apps/demo/components/TestingUtilitiesDemo.tsx around lines 178 to 180, the
code calls global.gc without safely ensuring it exists, which can cause errors
in environments where gc is not exposed. Update the check to confirm global.gc
is a function before calling it, and add a comment noting that this test
requires Node to be run with the --expose-gc flag to enable manual garbage
collection.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
CLAUDE.md (1)
140-148: React example still accesses a nonexistentcountproperty
TheCounterBlocemits a plainnumber, sostateitself is the count. Usingstate.countwill throw at runtime.This was flagged in the previous review and remains unresolved.
🧹 Nitpick comments (1)
CLAUDE.md (1)
54-70: Specify a language for the project-structure fenced block
markdownlint(MD040) warns because the fenced block beginning at Line 54 lacks a language identifier. Usetext(orbash) so tooling can parse it correctly.-``` +```text / ├── packages/ │ ├── blac/ # Core state management (@blac/core) ... └── tsconfig.base.json # Base TypeScript configuration</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 2f8701b3235033f263c3dcac7fe8dcd736d4c7c0 and cef7c799c570a65b82be7097fbd5265b10ba734c. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `CLAUDE.md` (1 hunks) * `apps/demo/components/RerenderLoggingDemo.tsx` (1 hunks) * `apps/demo/components/TestingUtilitiesDemo.tsx` (1 hunks) * `apps/demo/components/ui/Input.tsx` (1 hunks) * `apps/demo/components/ui/Label.tsx` (1 hunks) * `apps/docs/eslint.config.mjs` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (5)</summary> * apps/demo/components/RerenderLoggingDemo.tsx * apps/demo/components/ui/Label.tsx * apps/demo/components/TestingUtilitiesDemo.tsx * apps/docs/eslint.config.mjs * apps/demo/components/ui/Input.tsx </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>CLAUDE.md</summary> 54-54: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
apps/playground/PLAYGROUND_FIX_SUMMARY.md (9)
24-27: Wording and clarity: avoid “to not rely”; clarify “empty strings for code”.
- Prefer “avoid relying” or “not to rely” over “to not rely”.
- “Demos now use empty strings for code” is ambiguous. Suggest explicitly stating the placeholder export and where real code comes from.
Apply:
- - Updated all demo index files to not rely on raw imports - - Demos now use empty strings for code, relying on the clean exports + - Updated all demo index files to avoid relying on raw imports + - Demo index files export an empty code placeholder, while the playground loads actual demo code from `demoCodeExports` + (e.g., `export const code = ''`), ensuring type consistency without bundling raw source
11-16: Be explicit about the export shape for detection.Clarify whether demos export a default component named App or a named export. This avoids ambiguity for consumers and aligns with the sandbox detector.
- - Each demo exports as `App` for consistent detection + - Each demo exports a default React component named `App` for consistent detection + (e.g., `export default function App() { ... }`)
29-32: Document the actual detection order/list or link to source.You mention a “cleaner list of possible component names,” but the list isn’t shown. Either include it or link to the detector in
sandbox.tsto keep docs and implementation in sync.Would you add the exact array/order used by the detector or a permalink to the function in
sandbox.ts?
48-54: Tailwind dependency implicit in className values.The replacements use Tailwind utility classes. If Tailwind isn’t available in the sandbox, styles won’t apply. Either confirm Tailwind is available or provide inline styles/basic HTML without relying on Tailwind.
| `<Badge>` | `<span className="inline-flex items-center px-2 py-1 rounded-full text-xs font-medium bg-gray-100">` | + + Note: The className examples assume Tailwind CSS is available in the sandbox. If it is not, consider either: + - Using inline styles for minimal presentation, or + - Including a minimal stylesheet loaded by the sandbox.
57-61: Link to MULTI_FILE_SUPPORT.md so readers can navigate.Convert the filename mention into an actual markdown link.
- Currently, the playground only supports single-file editing. See `MULTI_FILE_SUPPORT.md` for plans to add: + Currently, the playground only supports single-file editing. See [MULTI_FILE_SUPPORT.md](./MULTI_FILE_SUPPORT.md) for plans to add:
35-45: Add a minimal example of the generated code for clarity.A short snippet helps readers understand the output shape and imports at a glance.
Example (to add after Line 45):
// Generated (simplified) import React from 'react' import { createBloc } from '@acme/blac' // or your actual package name export default function App() { // self-contained implementation... return <div>Hello Playground</div> }I can draft a concrete example matching your processor’s output if you share the current generator function.
64-69: Expand testing steps to cover fallback path and console errors.Include steps to validate demos without clean exports and to check for runtime/TS errors.
3. Click "Open in Playground" 4. The code should load and be runnable without errors +5. Repeat with a demo that lacks a clean export to verify the fallback processor path +6. Inspect browser console and TypeScript diagnostics panel for errors/warnings
72-77: Use relative links for modified files and drop leading slashes.This improves readability and allows clickable links in most viewers.
-- `/apps/playground/src/pages/PlaygroundPage.tsx` - Updated to use clean code -- `/apps/playground/src/lib/demoCodeProcessor.ts` - New processor for cleaning demo code -- `/apps/playground/src/demos/demoCodeExports.ts` - Clean demo code exports -- `/apps/playground/src/lib/sandbox.ts` - Improved component detection +- apps/playground/src/pages/PlaygroundPage.tsx – Updated to use clean code +- apps/playground/src/lib/demoCodeProcessor.ts – New processor for cleaning demo code +- apps/playground/src/demos/demoCodeExports.ts – Clean demo code exports +- apps/playground/src/lib/sandbox.ts – Improved component detection - All demo `index.ts` files - Removed `?raw` imports
1-91: Overall: solid, actionable doc — consider adding permalinks and examples.The structure is clear and helpful. If you add:
- A permalink to the detector in
sandbox.ts,- A tiny “generated code” example,
- An explicit note about Tailwind availability,
this will reduce ambiguity for contributors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
apps/playground/MULTI_FILE_SUPPORT.md(1 hunks)apps/playground/PLAYGROUND_FIX_SUMMARY.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/playground/MULTI_FILE_SUPPORT.md
🧰 Additional context used
🪛 LanguageTool
apps/playground/PLAYGROUND_FIX_SUMMARY.md
[style] ~25-~25: Consider changing the order of words to improve your wording.
Context: ...t errors - Updated all demo index files to not rely on raw imports - Demos now use emp...
(TO_NOT_VB)
🔇 Additional comments (1)
apps/playground/PLAYGROUND_FIX_SUMMARY.md (1)
41-43: No changes needed for brand capitalization
The occurrences of “BlaC,” “Bloc,” and “BLoC” each serve a distinct purpose and are used consistently across the repo:
- BlaC is the library’s brand name.
- Bloc is the name of the core class in the API (
class Bloc<S, E>) and its lowercase usage in imports (@blac/core).- BLoC (all-caps O) refers to the generic “Business Logic Component” pattern by acronym.
No files require renaming or capitalization adjustments.
Likely an incorrect or invalid review comment.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
apps/docs/getting-started/learning-paths.md (6)
5-5: Fix anchor slugs by removing emojis from headings (resolves MD051)
Emojis in headings make the “Next Steps” anchor fragments unreliable. Drop them to generate predictable slugs like #quick-start-path, #advanced-path, etc.-## 🚀 Quick Start Path +## Quick Start Path -## 📚 Fundamentals Path +## Fundamentals Path -## 🔥 Advanced Path +## Advanced Path -## 🎯 Migration Path +## Migration Path -## 📖 Reference Path +## Reference Path -## 🚢 Production Path +## Production Path -## 🎓 Interactive Learning +## Interactive LearningAlso applies to: 25-25, 48-48, 101-101, 126-126, 147-147, 173-173
7-7: Replace emphasis used as headings (MD036) with proper subheadings
Convert bold sentences used as pseudo-headings into H3s for accessibility and consistent ToC structure.-**For developers who want to get up and running quickly** +### Who is this for? +For developers who want to get up and running quickly -**For developers who want to understand core concepts** +### Who is this for? +For developers who want to understand core concepts -**For developers building complex applications** +### Who is this for? +For developers building complex applications -**For teams migrating from other state management solutions** +### Who is this for? +For teams migrating from other state management solutions -**For quick lookups and API reference** +### Purpose +For quick lookups and API reference -**For teams preparing for production** +### Who is this for? +For teams preparing for productionAlso applies to: 27-27, 50-50, 103-103, 128-128, 149-149
193-203: Use a native admonition instead of raw HTML; also fixes MD036/inline styles
Switch the custom HTML tip to a VitePress-style admonition for consistent theming and linting compliance.-<div class="tip custom-block" style="padding-top: 8px"> - -**New to BlaC?** Start with the [Quick Start Path](#quick-start-path) - -**Know the basics?** Jump to the [Advanced Path](#advanced-path) - -**Migrating a project?** Check the [Migration Path](#migration-path) - -**Need specific info?** Use the [Reference Path](#reference-path) - -</div> +::: tip +New to BlaC? Start with the [Quick Start Path](#quick-start-path) + +Know the basics? Jump to the [Advanced Path](#advanced-path) + +Migrating a project? Check the [Migration Path](#migration-path) + +Need specific info? Use the [Reference Path](#reference-path) +:::
59-63: Minor copy: “Prop-based Blocs” reads better than “Props-based Blocs”
Small terminology polish to match common React phrasing.- - Props-based Blocs for dynamic instances + - Prop-based Blocs for dynamic instances
189-208: Add explicit call-to-action links for each “Next Steps” path
Consider adding direct links under “Next Steps” beyond anchors to help users jump to separate pages (Quick Start, Advanced, Migration, Reference) if they arrive without page anchors.I can add a compact list of outbound links under the tip if desired.
68-68: Add link to the RenderLoggingPlugin package in the docsThe
RenderLoggingPluginis exported from@blac/plugin-render-logging(see packages/plugins/system/render-logging/src/index.ts), so the mentions in apps/docs/getting-started/learning-paths.md should point to the published package.• apps/docs/getting-started/learning-paths.md line 67
• apps/docs/getting-started/learning-paths.md line 168Suggested change (repeat at both locations):
- - Using the RenderLoggingPlugin + - [Using the RenderLoggingPlugin](https://www.npmjs.com/package/@blac/plugin-render-logging)apps/docs/.vitepress/config.mts (3)
161-164: Avoid branch drift in edit links by using HEAD.Hardcoding
maincan cause “Edit this page” to point to the wrong branch when viewing docs from release branches.Apply:
- pattern: 'https://github.com/jsnanigans/blac/edit/main/apps/docs/:path', + pattern: 'https://github.com/jsnanigans/blac/edit/HEAD/apps/docs/:path',If you prefer to lock to a specific branch (e.g., docs deploy from
main), keepmain. OtherwiseHEADis safer across branches.
10-16: Enhance social and SEO metadata for richer link previewsAll referenced local assets (
favicon.svg,logo.svg) exist inapps/docs/public/. Consider the following optional improvements to maximize compatibility and preview richness:• Use separate light/dark
theme-colorentries for better UA theming.
• Switch to absolute URLs and PNG/JPG assets for Open Graph/Twitter images.
• Add key Open Graph and Twitter meta tags (og:title,og:description,og:url).
• Upgrade Twitter card tosummary_large_imagefor larger previews.Suggested diff:
- ['meta', { name: 'theme-color', content: '#61dafb' }], - ['meta', { name: 'og:type', content: 'website' }], - ['meta', { name: 'og:site_name', content: 'BlaC' }], - ['meta', { name: 'og:image', content: '/logo.svg' }], - ['meta', { name: 'twitter:card', content: 'summary' }], - ['meta', { name: 'twitter:image', content: '/logo.svg' }], + // Theme color per scheme (Chromium-based browsers + Android) + ['meta', { name: 'theme-color', media: '(prefers-color-scheme: light)', content: '#61dafb' }], + ['meta', { name: 'theme-color', media: '(prefers-color-scheme: dark)', content: '#0d1117' }], + + // Open Graph metadata + ['meta', { property: 'og:type', content: 'website' }], + ['meta', { property: 'og:site_name', content: 'BlaC' }], + ['meta', { property: 'og:title', content: 'BlaC' }], + ['meta', { property: 'og:description', content: 'Business Logic as Components – Simple, powerful state management that separates business logic from UI. Type-safe, testable, and scales with your React application.' }], + // Replace with your deployed PNG/JPG asset + ['meta', { property: 'og:image', content: 'https://your-domain/og-image.png' }], + // Optional canonical URL + // ['link', { rel: 'canonical', href: 'https://your-domain' }], + + // Twitter Card metadata + ['meta', { name: 'twitter:card', content: 'summary_large_image' }], + ['meta', { name: 'twitter:image', content: 'https://your-domain/og-image.png' }],
150-154: Remove unsupportedsearch.options.detailedViewThe
detailedViewoption is not listed in the VitePress 1.6.3 search configuration (nowhere in the official docs forthemeConfig.search), so it will be ignored at runtime. Since you’re on VitePress ^1.6.3, you can safely remove this option:• In
apps/docs/.vitepress/config.mtsat lines 150–154, delete theoptions.detailedView: trueentry.
• Keepprovider: 'local'— that is correct for enabling the built-in local search.If you intended to tweak the search modal’s presentation, consult the supported config keys in the Default Theme Search reference (e.g. custom renderers, MiniSearch options, or translation overrides) instead of
detailedView.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/docs/.vitepress/config.mts(1 hunks)apps/docs/getting-started/learning-paths.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/docs/getting-started/learning-paths.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...s and use cases. ## 🚀 Quick Start Path **For developers who want to get up and run...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...erns in action. ## 📚 Fundamentals Path **For developers who want to understand cor...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...roduction) - Philosophy and architecture - [State Management](/concepts/state-manage...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...ate-management) - How BlaC manages state - Cubits - Simple state...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ncepts/cubits) - Simple state containers - [Your First Cubit](/getting-started/first...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...s/blocs) - Event-driven state management - [Your First Bloc](/getting-started/first-...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...-bloc) - Build an event-driven component - [Async Operations](/getting-started/async...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...ooks) - useBloc and useExternalBlocStore - React Patterns - Best ...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...atterns) - Best practices for React apps - [Instance Management](/concepts/instance-...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ... isolated instances ## 🔥 Advanced Path **For developers building complex applicati...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ...timization 1. Selectors & Memoization - Understanding proxy-based dependency tra...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...standing proxy-based dependency tracking - Writing efficient selectors - Prevent...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...racking - Writing efficient selectors - Preventing unnecessary re-renders 2. **...
(QB_NEW_EN)
[grammar] ~60-~60: There might be a mistake here.
Context: ...When to use shared vs isolated instances - Keep-alive patterns for persistent state...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...Keep-alive patterns for persistent state - Props-based Blocs for dynamic instances ...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...amic instances 3. Testing & Debugging - Unit testing Cubits and Blocs - Integ...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...ing** - Unit testing Cubits and Blocs - Integration testing with React - Usin...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...locs - Integration testing with React - Using the RenderLoggingPlugin ### Plugi...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...Development 1. Plugin System Overview - [Understanding the plugin architecture](/...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ...- Understanding the plugin architecture - [System vs Bloc plugins](/plugins/system-...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...s/system-plugins) 2. Built-in Plugins - [Persistence Plugin](/plugins/persistence...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...(/plugins/persistence) - Auto-save state - Creating custom logging plugins 3. **Cu...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ... plugins 3. Custom Plugin Development - [Creating your own plugins](/plugins/crea...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...opment** - Creating your own plugins - Lifecycle hooks and events - Best pra...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...plugins) - Lifecycle hooks and events - Best practices ### Architecture Pattern...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...ture Patterns 1. Composition Patterns - Composing multiple Blocs - Parent-chi...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...cs - Parent-child state relationships - Cross-cutting concerns 2. **Error Handl...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...ss-cutting concerns 2. Error Handling - Global error boundaries - Bloc-level ...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ... Handling** - Global error boundaries - Bloc-level error handling - Recovery ...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...oundaries - Bloc-level error handling - Recovery strategies 3. **Real-World Pat...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...ery strategies 3. Real-World Patterns - Authentication flows - Form managemen...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...rld Patterns** - Authentication flows - Form management - Data fetching and c...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...uthentication flows - Form management - Data fetching and caching ## 🎯 Migrati...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...tching and caching ## 🎯 Migration Path **For teams migrating from other state mana...
(QB_NEW_EN)
[grammar] ~126-~126: There might be a mistake here.
Context: ...tion/from-context) ## 📖 Reference Path For quick lookups and API reference ##...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...](/api/core/blac) - Global configuration - Cubit - Simple state c...
(QB_NEW_EN)
[grammar] ~133-~133: There might be a mistake here.
Context: ...api/core/cubit) - Simple state container - Bloc - Event-driven con...
(QB_NEW_EN)
[grammar] ~134-~134: There might be a mistake here.
Context: .../api/core/bloc) - Event-driven container - BlocBase - Base cl...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ...api/react/use-bloc) - Primary React hook - [useExternalBlocStore](/api/react/use-ext...
(QB_NEW_EN)
[grammar] ~144-~144: There might be a mistake here.
Context: ...### Plugin APIs - Plugin API Reference - [Creating Plugins](/plugins/creating-plug...
(QB_NEW_EN)
[grammar] ~147-~147: There might be a mistake here.
Context: ...creating-plugins) ## 🚢 Production Path For teams preparing for production ###...
(QB_NEW_EN)
[grammar] ~153-~153: There might be a mistake here.
Context: ... [ ] Error handling strategy implemented - [ ] Performance monitoring in place - [ ...
(QB_NEW_EN)
[grammar] ~154-~154: There might be a mistake here.
Context: ...ed - [ ] Performance monitoring in place - [ ] State persistence configured - [ ] T...
(QB_NEW_EN)
[grammar] ~155-~155: There might be a mistake here.
Context: ...place - [ ] State persistence configured - [ ] Testing coverage > 80% - [ ] Documen...
(QB_NEW_EN)
[grammar] ~156-~156: There might be a mistake here.
Context: ... configured - [ ] Testing coverage > 80% - [ ] Documentation updated ### Best Prac...
(QB_NEW_EN)
[grammar] ~161-~161: There might be a mistake here.
Context: ...## Best Practices - Testing strategies - [Performance optimization](/patterns/perf...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ...rns/testing) - Performance optimization - [Error handling](/patterns/error-handling...
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: .../patterns/performance) - Error handling - [State persistence](/patterns/persistence...
(QB_NEW_EN)
[grammar] ~168-~168: There might be a mistake here.
Context: ...bugging - Using the RenderLoggingPlugin - Performance profiling - State debugging ...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...derLoggingPlugin - Performance profiling - State debugging techniques - Production ...
(QB_NEW_EN)
[grammar] ~170-~170: There might be a mistake here.
Context: ...e profiling - State debugging techniques - Production error tracking ## 🎓 Interac...
(QB_NEW_EN)
[grammar] ~173-~173: There might be a mistake here.
Context: ...ror tracking ## 🎓 Interactive Learning ### Try the Playground The [BlaC Playground]...
(QB_NEW_EN)
[grammar] ~185-~185: There might be a mistake here.
Context: ...nigans/blac/discussions) - Ask questions - Discord Community - Chat with other ...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ...mmunity](#) - Chat with other developers - [Stack Overflow](https://stackoverflow.co...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
apps/docs/getting-started/learning-paths.md
7-7: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
27-27: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
50-50: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
103-103: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
128-128: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
149-149: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
186-186: No empty links
(MD042, no-empty-links)
195-195: Link fragments should be valid
(MD051, link-fragments)
197-197: Link fragments should be valid
(MD051, link-fragments)
199-199: Link fragments should be valid
(MD051, link-fragments)
201-201: Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (10)
apps/docs/getting-started/learning-paths.md (1)
69-83: Plugin documentation verified – links are valid and naming is up to date
All referenced “plugins” pages still exist in v2 and follow the current architecture. No renaming to “extensions” or “observers” is needed.• apps/docs/plugins/overview.md
• apps/docs/plugins/system-plugins.md
• apps/docs/plugins/persistence.md
• apps/docs/plugins/creating-plugins.md
• apps/docs/plugins/api-reference.mdapps/docs/.vitepress/config.mts (9)
6-8: Branding/description LGTM.Title and description read well and align with the BlaC positioning.
20-20: Site title LGTM.Consistent with the brand. No action.
23-26: Top nav LGTM, but verify target pages exist.The slugs look right; just ensure
/api/core/blacexists in the docs tree.Run the link check script in the sidebar comment below to verify this and all sidebar entries.
146-146: Social links LGTM.Direct link to the repo is correct.
158-159: Footer LGTM.Spans 2023–present; consistent with brand capitalization “BlaC”.
175-179: Markdown theme LGTM.Light/dark set to GitHub schemes; aligns with the added theme-color meta above.
184-184: Good composition withwithMermaid.Spreading
siteConfigkeeps the base config clean and the Mermaid-specific options colocated with the wrapper.
242-242: Mermaid plugin class LGTM.Standard
'mermaid'class keeps styling straightforward and avoids custom CSS surprises.
2-2: Pin Versions Confirmed in apps/docsThe
vitepressandvitepress-plugin-mermaiddependencies are only declared in apps/docs/package.json at versions^1.6.3and^2.0.17. No other package.json in the repo lists these dependencies.
- If caret (
^) ranges for patch updates are acceptable for your stability requirements, no further action is needed.- Otherwise, consider locking to exact patch versions (e.g.
1.6.3and2.0.17) or hoisting these deps into the monorepo root to enforce a single source of truth.
| sidebar: [ | ||
| { | ||
| text: 'Introduction', | ||
| items: [ | ||
| { text: 'Introduction', link: '/learn/introduction' }, | ||
| { text: 'Getting Started', link: '/learn/getting-started' }, | ||
| { text: 'Architecture', link: '/learn/architecture' }, | ||
| { text: 'Core Concepts', link: '/learn/core-concepts' } | ||
| ] | ||
| { text: 'What is BlaC?', link: '/introduction' }, | ||
| { text: 'Comparisons', link: '/comparisons' }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'Basic Usage', | ||
| text: 'Getting Started', | ||
| items: [ | ||
| { text: 'The Blac Pattern', link: '/learn/blac-pattern' }, | ||
| { text: 'State Management Patterns', link: '/learn/state-management-patterns' }, | ||
| { text: 'Best Practices', link: '/learn/best-practices' } | ||
| ] | ||
| { text: 'Learning Paths', link: '/getting-started/learning-paths' }, | ||
| { text: 'Installation', link: '/getting-started/installation' }, | ||
| { text: 'Your First Cubit', link: '/getting-started/first-cubit' }, | ||
| { | ||
| text: 'Async Operations', | ||
| link: '/getting-started/async-operations', | ||
| }, | ||
| { text: 'Your First Bloc', link: '/getting-started/first-bloc' }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'Core Concepts', | ||
| items: [ | ||
| { text: 'State Management', link: '/concepts/state-management' }, | ||
| { text: 'Cubits', link: '/concepts/cubits' }, | ||
| { text: 'Blocs', link: '/concepts/blocs' }, | ||
| { | ||
| text: 'Instance Management', | ||
| link: '/concepts/instance-management', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'API Reference', | ||
| items: [ | ||
| { text: 'Core Classes', link: '/api/core-classes' }, | ||
| { text: 'React Hooks', link: '/api/react-hooks' }, | ||
| { text: 'Key Methods', link: '/api/key-methods' } | ||
| ] | ||
| { | ||
| text: '@blac/core', | ||
| collapsed: false, | ||
| items: [ | ||
| { text: 'Blac', link: '/api/core/blac' }, | ||
| { text: 'Cubit', link: '/api/core/cubit' }, | ||
| { text: 'Bloc', link: '/api/core/bloc' }, | ||
| { text: 'BlocBase', link: '/api/core/bloc-base' }, | ||
| ], | ||
| }, | ||
| { | ||
| text: '@blac/react', | ||
| collapsed: false, | ||
| items: [ | ||
| { text: 'useBloc', link: '/api/react/use-bloc' }, | ||
| { | ||
| text: 'useExternalBlocStore', | ||
| link: '/api/react/use-external-bloc-store', | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'React Integration', | ||
| items: [ | ||
| { text: 'Hooks', link: '/react/hooks' }, | ||
| { text: 'Patterns', link: '/react/patterns' }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'Plugin System', | ||
| items: [ | ||
| { text: 'Overview', link: '/plugins/overview' }, | ||
| { text: 'System Plugins', link: '/plugins/system-plugins' }, | ||
| { text: 'Bloc Plugins', link: '/plugins/bloc-plugins' }, | ||
| { text: 'Creating Plugins', link: '/plugins/creating-plugins' }, | ||
| { text: 'Persistence Plugin', link: '/plugins/persistence' }, | ||
| { text: 'API Reference', link: '/plugins/api-reference' }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'Patterns & Recipes', | ||
| collapsed: true, | ||
| items: [ | ||
| { text: 'Testing', link: '/patterns/testing' }, | ||
| { text: 'Persistence', link: '/patterns/persistence' }, | ||
| { text: 'Error Handling', link: '/patterns/error-handling' }, | ||
| { text: 'Performance', link: '/patterns/performance' }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'Examples', | ||
| items: [ | ||
| { text: 'Counter', link: '/examples/counter' }, | ||
| { text: 'Todo List', link: '/examples/todo-list' }, | ||
| { text: 'Authentication', link: '/examples/authentication' }, | ||
| ], | ||
| }, | ||
| { | ||
| text: 'Legacy', | ||
| collapsed: true, | ||
| items: [ | ||
| { text: 'Old Introduction', link: '/learn/introduction' }, | ||
| { text: 'Old Getting Started', link: '/learn/getting-started' }, | ||
| { text: 'Old Architecture', link: '/learn/architecture' }, | ||
| { text: 'Old Core Concepts', link: '/learn/core-concepts' }, | ||
| { text: 'The Blac Pattern', link: '/learn/blac-pattern' }, | ||
| { | ||
| text: 'State Management Patterns', | ||
| link: '/learn/state-management-patterns', | ||
| }, | ||
| { text: 'Best Practices', link: '/learn/best-practices' }, | ||
| { text: 'Old Core Classes', link: '/api/core-classes' }, | ||
| { text: 'Old React Hooks', link: '/api/react-hooks' }, | ||
| { text: 'Key Methods', link: '/api/key-methods' }, | ||
| { text: 'Configuration', link: '/api/configuration' }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify all sidebar routes resolve to files.
There’s a sizable set of new/relocated pages. Let’s pre-empt broken links (build-time warnings don’t always cover all cases).
Use this script to flag missing targets. It checks for either :slug.md or :slug/index.md under apps/docs/.
If any are missing, I can auto-generate stubs with frontmatter to unblock the site build.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
root="apps/docs"
check() {
local slug="$1"
local f1="$root${slug}.md"
local f2="$root${slug}/index.md"
if [[ -f "$f1" || -f "$f2" ]]; then
printf "OK %s -> %s\n" "$slug" "$( [ -f "$f1" ] && echo "$f1" || echo "$f2" )"
else
printf "MISSING %s -> (%s | %s)\n" "$slug" "$f1" "$f2"
missing=1
fi
}
missing=0
slugs=(
/introduction
/comparisons
/getting-started/learning-paths
/getting-started/installation
/getting-started/first-cubit
/getting-started/async-operations
/getting-started/first-bloc
/concepts/state-management
/concepts/cubits
/concepts/blocs
/concepts/instance-management
/api/core/blac
/api/core/cubit
/api/core/bloc
/api/core/bloc-base
/api/react/use-bloc
/api/react/use-external-bloc-store
/react/hooks
/react/patterns
/plugins/overview
/plugins/system-plugins
/plugins/bloc-plugins
/plugins/creating-plugins
/plugins/persistence
/plugins/api-reference
/patterns/testing
/patterns/persistence
/patterns/error-handling
/patterns/performance
/examples/counter
/examples/todo-list
/examples/authentication
/learn/introduction
/learn/getting-started
/learn/architecture
/learn/core-concepts
/learn/blac-pattern
/learn/state-management-patterns
/learn/best-practices
/api/core-classes
/api/react-hooks
/api/key-methods
/api/configuration
)
for s in "${slugs[@]}"; do check "$s"; done
exit $missingLength of output: 21556
Missing sidebar routes detected – critical fixes required
The following sidebar entries have no corresponding Markdown source under apps/docs/, causing broken links:
/api/core/bloc-base/api/react/use-bloc/patterns/testing/patterns/persistence/patterns/error-handling/patterns/performance
Please add the missing files (either as :slug.md or :slug/index.md) for each route. If you’d like, I can help by auto-generating lightweight stub files with frontmatter to unblock the build.
🤖 Prompt for AI Agents
In apps/docs/.vitepress/config.mts around lines 28 to 142, the sidebar contains
routes that don't have corresponding Markdown sources, causing broken links for
/api/core/bloc-base, /api/react/use-bloc, and the four /patterns/* pages; create
the missing Markdown files (either slug.md or slug/index.md) under apps/docs
matching those paths with minimal frontmatter (title, description) and a short
stub body, or add redirects/remove the sidebar entries if the pages are
intentionally omitted; ensure filenames and directory structure exactly match
the links so the VitePress build resolves them.
| lastUpdated: { | ||
| text: 'Updated at', | ||
| formatOptions: { | ||
| dateStyle: 'short', | ||
| timeStyle: 'medium', | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Enable git timestamps to actually show “Updated at”.
Theme config alone isn’t enough; VitePress needs the top-level lastUpdated: true to extract git timestamps during build.
Apply:
-const siteConfig = defineConfig({
+const siteConfig = defineConfig({
+ lastUpdated: true,This prevents the “Updated at” label from rendering without a date.
📝 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.
| lastUpdated: { | |
| text: 'Updated at', | |
| formatOptions: { | |
| dateStyle: 'short', | |
| timeStyle: 'medium', | |
| }, | |
| }, | |
| }, | |
| // apps/docs/.vitepress/config.mts | |
| import { defineConfig } from 'vitepress' | |
| const siteConfig = defineConfig({ | |
| lastUpdated: true, // ← enable git‐based timestamps | |
| // …other top-level config options… | |
| themeConfig: { | |
| // …other theme config… | |
| lastUpdated: { | |
| text: 'Updated at', | |
| formatOptions: { | |
| dateStyle: 'short', | |
| timeStyle: 'medium', | |
| }, | |
| }, | |
| }, | |
| }) | |
| export default siteConfig |
🤖 Prompt for AI Agents
In apps/docs/.vitepress/config.mts around lines 166 to 173, the theme's
lastUpdated object is configured but VitePress requires a top-level lastUpdated:
true to extract git timestamps; add a top-level property lastUpdated: true in
this config (alongside existing theme config) so the “Updated at” label renders
actual dates during build.
| theme: 'base', | ||
| themeVariables: { | ||
| // --- Core Colors for Neon Look --- | ||
| primaryColor: '#00BFFF', // DeepSkyBlue (Vibrant Blue for main nodes) | ||
| primaryTextColor: '#FFFFFF', // White text on nodes | ||
| primaryBorderColor: '#FF00FF', // Neon Magenta (for borders, "cute" pop) | ||
|
|
||
| lineColor: '#39FF14', // Neon Green (for arrows, connectors) | ||
| textColor: '#E0E0E0', // Light Grey for general text/labels | ||
|
|
||
| // --- Backgrounds --- | ||
| mainBkg: '#1A1A1A', // Very Dark Grey (to make neons pop) | ||
| clusterBkg: '#242424', // Dark Grey Soft (for subgraphs) | ||
| clusterBorderColor: '#00BFFF', // DeepSkyBlue border for clusters | ||
|
|
||
| // --- Node specific (if not covered by primary) --- | ||
| // These often inherit from primary, but can be set explicitly | ||
| nodeBorder: '#FF00FF', // Consistent with primaryBorderColor (Neon Magenta) | ||
| nodeTextColor: '#FFFFFF', // Consistent with primaryTextColor (White) | ||
|
|
||
| // --- Accents & Special Elements: "Cute Neon" Notes --- | ||
| noteBkgColor: '#2c003e', // Deep Dark Magenta/Purple base for notes | ||
| noteTextColor: '#FFFFA0', // Neon Pale Yellow text on notes (cute & readable) | ||
| noteBorderColor: '#FF00AA', // Bright Neon Pink border for notes | ||
|
|
||
| // --- For Sequence Diagrams (Neon) --- | ||
| actorBkg: '#39FF14', // Neon Green for actor boxes | ||
| actorBorder: '#2E8B57', // SeaGreen (darker green border for actors for definition) | ||
| actorTextColor: '#000000', // Black text on Neon Green actors for contrast | ||
|
|
||
| signalColor: '#FF00FF', // Neon Magenta for signal lines | ||
| signalTextColor: '#FFFFFF', // White text on signal lines | ||
|
|
||
| labelBoxBkgColor: '#BF00FF', // Electric Purple for label boxes (like 'loop', 'alt') | ||
| labelTextColor: '#FFFFFF', // White text on label boxes | ||
|
|
||
| sequenceNumberColor: '#FFFFFF', // White for sequence numbers for visibility on dark lanes | ||
|
|
||
| // --- Fonts - aligning with common VitePress/modern web defaults --- | ||
| fontFamily: '"Inter", -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen, Ubuntu, Cantarell, "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif', | ||
| fontSize: '22px', // Standard readable size | ||
| } | ||
| // Clean, minimal theme | ||
| primaryColor: '#61dafb', | ||
| primaryTextColor: '#fff', | ||
| primaryBorderColor: '#4db8d5', | ||
| lineColor: '#5e6c84', | ||
| secondaryColor: '#f4f5f7', | ||
| tertiaryColor: '#e3e4e6', | ||
| background: '#ffffff', | ||
| mainBkg: '#61dafb', | ||
| secondBkg: '#f4f5f7', | ||
| tertiaryBkg: '#e3e4e6', | ||
| secondaryBorderColor: '#c1c7d0', | ||
| tertiaryBorderColor: '#d3d5d9', | ||
| secondaryTextColor: '#172b4d', | ||
| tertiaryTextColor: '#42526e', | ||
| textColor: '#172b4d', | ||
| mainContrastColor: '#172b4d', | ||
| darkTextColor: '#172b4d', | ||
| border1: '#4db8d5', | ||
| border2: '#c1c7d0', | ||
| arrowheadColor: '#5e6c84', | ||
| fontFamily: | ||
| '-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif', | ||
| fontSize: '16px', | ||
| labelBackground: '#f4f5f7', | ||
| nodeBkg: '#61dafb', | ||
| nodeBorder: '#4db8d5', | ||
| clusterBkg: '#f4f5f7', | ||
| clusterBorder: '#c1c7d0', | ||
| defaultLinkColor: '#5e6c84', | ||
| titleColor: '#172b4d', | ||
| edgeLabelBackground: '#ffffff', | ||
| actorBorder: '#4db8d5', | ||
| actorBkg: '#61dafb', | ||
| actorTextColor: '#ffffff', | ||
| actorLineColor: '#5e6c84', | ||
| signalColor: '#172b4d', | ||
| signalTextColor: '#172b4d', | ||
| labelBoxBkgColor: '#61dafb', | ||
| labelBoxBorderColor: '#4db8d5', | ||
| labelTextColor: '#ffffff', | ||
| loopTextColor: '#172b4d', | ||
| noteBorderColor: '#c1c7d0', | ||
| noteBkgColor: '#fff8dc', | ||
| noteTextColor: '#172b4d', | ||
| activationBorderColor: '#172b4d', | ||
| activationBkgColor: '#f4f5f7', | ||
| sequenceNumberColor: '#ffffff', | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Right idea; consider dark-mode theming + reduce brittle Mermaid variables.
- Mermaid’s
themeVariablessurface changes over time; many keys are ignored between versions. Minimizing to the essentials reduces breakage. - Current palette uses white text on
#61dafb. That contrast may be borderline depending on font size. - Consider a dark-mode variant to match VitePress’ dark theme.
Two incremental improvements:
- Use a smaller, stable subset and CSS variables for easy tuning:
mermaid: {
- theme: 'base',
- themeVariables: {
- // Clean, minimal theme
- primaryColor: '#61dafb',
- primaryTextColor: '#fff',
- primaryBorderColor: '#4db8d5',
- lineColor: '#5e6c84',
- secondaryColor: '#f4f5f7',
- tertiaryColor: '#e3e4e6',
- background: '#ffffff',
- ...
- },
+ theme: 'base',
+ themeVariables: {
+ primaryColor: 'var(--vp-c-brand-1)',
+ primaryTextColor: 'var(--vp-c-text-inverse)',
+ lineColor: 'var(--vp-c-border)',
+ background: 'var(--vp-c-bg)',
+ textColor: 'var(--vp-c-text-1)',
+ fontFamily: 'var(--vp-font-family-base)',
+ fontSize: '16px'
+ },- Optional dark override via CSS (keeps config static):
/* apps/docs/.vitepress/theme/mermaid.css */
:root .mermaid {
--vp-c-brand-1: #61dafb;
}
.dark .mermaid {
--vp-c-brand-1: #3fb0d4; /* slightly darker for contrast on dark bg */
}Then import ./theme/mermaid.css in .vitepress/theme/index.ts.
Quickly sanity-check WCAG contrast for key pairs:
🏁 Script executed:
#!/bin/bash
python - "$@" << 'PY'
def hex2rgb(h): h=h.lstrip('#'); return tuple(int(h[i:i+2],16)/255 for i in (0,2,4))
def rel_lum(c):
def f(u): return u/12.92 if u<=0.03928 else ((u+0.055)/1.055)**2.4
r,g,b=c; r,g,b = f(r),f(g),f(b); return 0.2126*r+0.7152*g+0.0722*b
def contrast(a,b):
L1,L2=sorted([rel_lum(a),rel_lum(b)], reverse=True)
return (L1+0.05)/(L2+0.05)
pairs = [
("#61dafb","#ffffff","primaryColor vs primaryTextColor"),
("#61dafb","#172b4d","primaryColor vs textColor"),
("#ffffff","#172b4d","background vs textColor (light)")
]
for fg,bg,label in pairs:
import math
c=contrast(hex2rgb(fg),hex2rgb(bg))
print(f"{label}: {c:.2f}:1")
PYLength of output: 126
Accessibility: Fix low contrast on primary text
Verified that the current pairing of primaryColor #61dafb and primaryTextColor #ffffff yields only a 1.62:1 contrast ratio—well below the WCAG minimum of 4.5:1 for normal text. To address this and keep your Mermaid config resilient:
• apps/docs/.vitepress/config.mts (lines 188–238):
– Reduce themeVariables to a minimal, stable subset
– Switch primaryTextColor to a darker CSS variable for compliance
• apps/docs/.vitepress/theme/mermaid.css:
– Maintain a static light-mode config and add a dark-mode override for brand tint
Proposed diff (highlighting the crucial change for contrast):
mermaid: {
- theme: 'base',
- themeVariables: {
- primaryColor: 'var(--vp-c-brand-1)',
- primaryTextColor: 'var(--vp-c-text-inverse)',
+ theme: 'base',
+ themeVariables: {
+ primaryColor: 'var(--vp-c-brand-1)',
+ primaryTextColor: 'var(--vp-c-text-1)', // #172b4d on #61dafb = 8.68:1
lineColor: 'var(--vp-c-border)',
background: 'var(--vp-c-bg)',
textColor: 'var(--vp-c-text-1)',
fontFamily: 'var(--vp-font-family-base)',
fontSize: '16px'
},And retain the optional CSS override for dark mode:
/* apps/docs/.vitepress/theme/mermaid.css */
:root .mermaid {
--vp-c-brand-1: #61dafb;
}
.dark .mermaid {
--vp-c-brand-1: #3fb0d4; /* slightly darker for contrast on dark bg */
}With these changes, you’ll both simplify upkeep (fewer brittle keys) and ensure diagrams meet accessibility standards.
| - [React Hooks](/react/hooks) - useBloc and useExternalBlocStore | ||
| - [React Patterns](/react/patterns) - Best practices for React apps |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify React hooks docs path; consider linking concrete APIs
Current link points to “/react/hooks” while other entries live under “/api/react/...”. Ensure the target exists or link directly to the primary hooks.
-- [React Hooks](/react/hooks) - useBloc and useExternalBlocStore
+- React Hooks — [useBloc](/api/react/use-bloc) and [useExternalBlocStore](/api/react/use-external-bloc-store)🏁 Script executed:
#!/bin/bash
# Verify existence of the linked pages/anchors in the docs repo.
# Looks for headings or files that likely map to the referenced pages.
echo "Checking for React hooks docs..."
rg -nC2 -i $'^#{1,6}\\s+React Hooks\\b' || true
fd -t f -a -e md -E "node_modules" | rg -n "/react/hooks|api/react/use-bloc|api/react/use-external-bloc-store" || trueLength of output: 1121
Update React Hooks link to point at the API reference (and fix missing “useBloc” page)
– In apps/docs/getting-started/learning-paths.md (lines 44–45), the entry currently reads:
- [React Hooks](/react/hooks) – useBloc and useExternalBlocStorebut the dedicated API reference lives at /api/react/hooks (apps/docs/api/react/hooks.md) and the individual “useExternalBlocStore” page exists at /api/react/use-external-bloc-store (apps/docs/api/react/use-external-bloc-store.md), whereas there is no /api/react/use-bloc page.
– Mandatory refactor: either
• Link the aggregate Hooks API and keep both hook names plain, for example:
diff - [React Hooks](/react/hooks) – useBloc and useExternalBlocStore + [React Hooks](/api/react/hooks) – useBloc and useExternalBlocStore
Verified: apps/docs/api/react/hooks.md exists, as does use-external-bloc-store.md; use-bloc.md does not.
• Or create apps/docs/api/react/use-bloc.md before using a direct link, and then update to:
diff - [React Hooks](/react/hooks) – useBloc and useExternalBlocStore + React Hooks — [useBloc](/api/react/use-bloc) and [useExternalBlocStore](/api/react/use-external-bloc-store)
Please adjust the links accordingly to avoid broken references.
🧰 Tools
🪛 LanguageTool
[grammar] ~44-~44: There might be a mistake here.
Context: ...ooks) - useBloc and useExternalBlocStore - React Patterns - Best ...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...atterns) - Best practices for React apps - [Instance Management](/concepts/instance-...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In apps/docs/getting-started/learning-paths.md around lines 44–45, the current
React Hooks entry links to /react/hooks but references individual hooks where
use-bloc page is missing; update the entry to point the main link to the API
reference (/api/react/hooks) and keep the hook names plain except for the
existing useExternalBlocStore which should link to
/api/react/use-external-bloc-store (or alternatively create
apps/docs/api/react/use-bloc.md before linking directly to use-bloc). Ensure no
link targets a nonexistent /api/react/use-bloc to avoid broken references.
| ### Community Resources | ||
|
|
||
| - [GitHub Discussions](https://github.com/jsnanigans/blac/discussions) - Ask questions | ||
| - [Discord Community](#) - Chat with other developers |
There was a problem hiding this comment.
Replace placeholder Discord link (MD042)
The empty link “#” breaks navigation and fails linting. Use the real invite URL or mark as “coming soon”.
-- [Discord Community](#) - Chat with other developers
+- Discord Community (coming soon) - Chat with other developers
+ <!-- TODO: Replace with invite URL when available -->If you have the invite URL, I can update it directly.
📝 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.
| - [Discord Community](#) - Chat with other developers | |
| - Discord Community (coming soon) - Chat with other developers | |
| <!-- TODO: Replace with invite URL when available --> |
🧰 Tools
🪛 LanguageTool
[grammar] ~186-~186: There might be a mistake here.
Context: ...mmunity](#) - Chat with other developers - [Stack Overflow](https://stackoverflow.co...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
186-186: No empty links
(MD042, no-empty-links)
🤖 Prompt for AI Agents
In apps/docs/getting-started/learning-paths.md around line 186 the Discord link
uses a placeholder "#" which breaks navigation and triggers MD042; replace the
placeholder with the actual Discord invite URL if available, or remove the link
and change the text to a non-linked label such as "Discord Community (coming
soon)" so linting passes and navigation is correct.
Summary by CodeRabbit