refactor: reset more global configurations in stories#646
Conversation
WalkthroughThe changes update documentation and reset configuration behavior. The Changes
Sequence Diagram(s)sequenceDiagram
participant Story as Story Execution
participant Reset as resetMaxGraphConfigs
participant Registry as CodecRegistry
participant ObjCodec as ObjectCodec
participant SheetCodec as StylesheetCodec
participant Stencil as StencilShapeRegistry
Story->>Reset: Invoke resetMaxGraphConfigs()
Reset->>Registry: Clear aliases and codecs
Reset->>ObjCodec: Restore allowEval from original config
Reset->>SheetCodec: Restore allowEval from original config
Reset->>Stencil: Reset stencils to {}
Reset->>Story: Return updated configuration
Possibly related PRs
Suggested labels
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This will limit the side effects of a story's modification of a global configuration on other stories. Do not reset the style registries (for shapes, perimeters, ...) at this time. Once they have been filled when initializing a Graph instance, they are not filled again when initializing a new Graph instance.
7a52f49 to
92f6c83
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/html/.storybook/preview.ts (1)
59-60: Consider consistent reset approach for all registries.You've implemented resets for stencil registries but noted in the PR objectives that style registries for shapes and perimeters won't be reset. This partial approach might lead to unpredictable behavior.
Consider creating a comprehensive tracking mechanism for all affected registries to ensure consistency. This could involve:
- Cataloging all registries that might be modified during story execution
- Implementing a systematic approach to handle different reset requirements
- Adding clear documentation about which registries are reset and which are not
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/serialization/codecs/StylesheetCodec.ts(1 hunks)packages/html/.storybook/preview.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/serialization/codecs/StylesheetCodec.ts
🔇 Additional comments (4)
packages/html/.storybook/preview.ts (4)
4-4: Good addition of necessary imports.The addition of codec and registry related imports is consistent with the PR objective to reset more global configurations.
Also applies to: 7-7, 16-17
33-36: Smart implementation for preserving original configuration.Capturing the initial state of
allowEvalproperties before they may be modified ensures proper restoration between stories. This is a good pattern for preserving the original configuration state.
50-53: Helpful documentation for future development.The comments clearly explain the current state of these registries and future intentions, which provides good context for maintainers.
54-58: Verify the safety of directly modifying registry objects.While this implementation achieves the goal of resetting configurations between stories, it relies on direct access to registry objects that are intended to be made private in the future.
Consider working with the core team to introduce proper public reset methods for these registries, as noted in your comments. This would provide a more maintainable solution when these properties become private.
This will limit the side effects of a story's modification of a global configuration on other stories.
Do not reset the style registries (for shapes, perimeters, ...) at this time. Once they have been filled when initializing a Graph instance, they are not filled again when initializing a new Graph instance.
Notes
Covers #418
Summary by CodeRabbit
Documentation
Chores