refactor: reset more global configurations in stories - #646
Conversation
Sorry, something went wrong.
WalkthroughThe changes update documentation and reset configuration behavior. The StylesheetCodec class now uses an @default tag to explicitly document that its allowEval property defaults to true, without affecting functionality. In addition, the Storybook preview file introduces new codec-related imports, defines a constant to store original evaluation settings, and enhances the resetMaxGraphConfigs function to clear codec registries and reset the allowEval properties and stencil registries to their initial states. 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 labelsdocumentation ✨ 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? ❤️ Share 🪧 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 (.coderabbit.yaml)
Documentation and Community
|
Sorry, something went wrong.
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.
|
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)packages/html/.storybook/preview.ts (1)📜 Review details59-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
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Reviewing files that changed from the base of the PR and between 1f415db and 92f6c83.
📒 Files selected for processing (2)- packages/core/src/serialization/codecs/StylesheetCodec.ts (1 hunks)
- packages/html/.storybook/preview.ts (4 hunks)
- packages/core/src/serialization/codecs/StylesheetCodec.ts
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 allowEval properties 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.
Sorry, something went wrong.
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