refactor: introduce internal log() shortcut function#974
Conversation
Add a convenient log() function in internal/utils.ts that returns GlobalConfig.logger. This provides a shorter, more readable alternative to accessing GlobalConfig.logger directly throughout the codebase. Also documented the pattern in CLAUDE.md for future contributors.
WalkthroughAdded a centralized exported Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts (1)
215-235: Fix the warning message: it logs the wrong value (templateobject) instead of the missing template key.
Suggested patch:- const create = () => { - const template = editor.templates[select.value]; + const create = () => { + const selectedTemplateName = select.value; + const template = editor.templates[selectedTemplateName]; if (template != null) { const clone = template.clone(); @@ return clone; } - log().warn(`Template ${template} not found`); + log().warn(`Template ${selectedTemplateName} not found`); return null; };
🧹 Nitpick comments (3)
packages/core/src/serialization/ObjectCodec.ts (2)
58-63: Docs updated tolog()— consider whether the example still matches the current API surface.
The snippet still referencesmxUtils.getPrettyXml(node); ifmxUtilsisn’t part of the public surface anymore, consider updating the example to the current equivalent (optional).Also applies to: 382-384, 676-678
448-449: Logging migration preserved behavior; optional micro-optimization: cache the logger in method scope.
If these warnings can trigger in loops/hot paths, considerconst logger = log();once per method before multiple log calls (purely optional).Also applies to: 527-528, 768-772
packages/core/src/view/layout/hierarchical/CoordinateAssignment.ts (1)
730-731: Warnings migrated; consider adding context to reduce “mystery logs”.
E.g., includenode.id/cell.ids?.[0]/ rank index in'edge.edges is null'/ bounds warnings.Also applies to: 745-746, 804-805, 826-827
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CLAUDE.md(2 hunks)packages/core/src/gui/MaxLog.ts(1 hunks)packages/core/src/internal/utils.ts(3 hunks)packages/core/src/serialization/Codec.ts(4 hunks)packages/core/src/serialization/ObjectCodec.ts(7 hunks)packages/core/src/serialization/codecs/StylesheetCodec.ts(2 hunks)packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts(2 hunks)packages/core/src/view/GraphView.ts(3 hunks)packages/core/src/view/layout/hierarchical/CoordinateAssignment.ts(6 hunks)packages/core/src/view/other/PrintPreview.ts(2 hunks)packages/core/src/view/plugins/ConnectionHandler.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/core/src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/core/src/**/*.ts: Include.jsfile extensions in all import statements within packages/core/src/ to comply with ESLint rulen/file-extension-in-import.
Do not use console statements in any code. Use proper logging mechanisms instead (ESLint rule:no-console: error).
Do not useeval()function in any code (ESLint rule:no-eval: error).
Do not use const enums. Convert all const enums to regular enums (ESLint rule forbids const enums).
TypeScript source files in core package must strictly maintain typing with no implicit types and all functions having explicit return type annotations.
Files:
packages/core/src/internal/utils.tspackages/core/src/view/other/PrintPreview.tspackages/core/src/serialization/codecs/StylesheetCodec.tspackages/core/src/view/layout/hierarchical/CoordinateAssignment.tspackages/core/src/gui/MaxLog.tspackages/core/src/serialization/ObjectCodec.tspackages/core/src/view/GraphView.tspackages/core/src/serialization/Codec.tspackages/core/src/view/plugins/ConnectionHandler.tspackages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use single quotes in TypeScript/JavaScript code (Prettier config:
singleQuote: true).
**/*.{ts,tsx,js,jsx}: UseisNullishfunction when checking fornullorundefinedon variables that can have falsy values (numbers, strings, booleans) to avoid treating0,"", orfalseas nullish.
Wrap multiple model changes inbatchUpdate()to ensure efficient batch processing.
Files:
packages/core/src/internal/utils.tspackages/core/src/view/other/PrintPreview.tspackages/core/src/serialization/codecs/StylesheetCodec.tspackages/core/src/view/layout/hierarchical/CoordinateAssignment.tspackages/core/src/gui/MaxLog.tspackages/core/src/serialization/ObjectCodec.tspackages/core/src/view/GraphView.tspackages/core/src/serialization/Codec.tspackages/core/src/view/plugins/ConnectionHandler.tspackages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with tab width 2, trailing comma ES5, print width 90, and end of line auto.
Files:
packages/core/src/internal/utils.tspackages/core/src/view/other/PrintPreview.tspackages/core/src/serialization/codecs/StylesheetCodec.tspackages/core/src/view/layout/hierarchical/CoordinateAssignment.tspackages/core/src/gui/MaxLog.tsCLAUDE.mdpackages/core/src/serialization/ObjectCodec.tspackages/core/src/view/GraphView.tspackages/core/src/serialization/Codec.tspackages/core/src/view/plugins/ConnectionHandler.tspackages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts
packages/core/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Define styles as objects conforming to
CellStyletype as documented inpackages/core/src/types.ts.
Files:
packages/core/src/internal/utils.tspackages/core/src/view/other/PrintPreview.tspackages/core/src/serialization/codecs/StylesheetCodec.tspackages/core/src/view/layout/hierarchical/CoordinateAssignment.tspackages/core/src/gui/MaxLog.tspackages/core/src/serialization/ObjectCodec.tspackages/core/src/view/GraphView.tspackages/core/src/serialization/Codec.tspackages/core/src/view/plugins/ConnectionHandler.tspackages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts
packages/core/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Code must conform to ES2020 standard and target modern browsers (Chrome, Edge, Firefox, Safari, Chromium-based browsers). No IE support.
Files:
packages/core/src/internal/utils.tspackages/core/src/view/other/PrintPreview.tspackages/core/src/serialization/codecs/StylesheetCodec.tspackages/core/src/view/layout/hierarchical/CoordinateAssignment.tspackages/core/src/gui/MaxLog.tspackages/core/src/serialization/ObjectCodec.tspackages/core/src/view/GraphView.tspackages/core/src/serialization/Codec.tspackages/core/src/view/plugins/ConnectionHandler.tspackages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: maxGraph/maxGraph PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-10T13:43:28.685Z
Learning: Applies to packages/core/src/**/*.ts : Do not use console statements in any code. Use proper logging mechanisms instead (ESLint rule: `no-console: error`).
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 740
File: packages/core/src/util/requestUtils.ts:79-106
Timestamp: 2025-03-31T08:32:17.055Z
Learning: PR #740 is focused on reorganizing code into namespaces (guiUtils and requestUtils) without changing any implementations. Implementation improvements should be suggested for future PRs.
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 849
File: packages/html/stories/DragSource.stories.js:98-101
Timestamp: 2025-06-13T07:48:10.300Z
Learning: User tbouffard prefers answers in English; avoid switching to other languages in future replies.
📚 Learning: 2025-12-10T13:43:28.685Z
Learnt from: CR
Repo: maxGraph/maxGraph PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-10T13:43:28.685Z
Learning: Applies to packages/core/src/**/*.ts : Do not use console statements in any code. Use proper logging mechanisms instead (ESLint rule: `no-console: error`).
Applied to files:
packages/core/src/internal/utils.tspackages/core/src/view/other/PrintPreview.tspackages/core/src/serialization/codecs/StylesheetCodec.tspackages/core/src/view/layout/hierarchical/CoordinateAssignment.tspackages/core/src/gui/MaxLog.tspackages/core/src/serialization/Codec.tspackages/core/src/view/plugins/ConnectionHandler.ts
📚 Learning: 2025-12-11T08:46:43.160Z
Learnt from: CR
Repo: maxGraph/maxGraph PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-11T08:46:43.160Z
Learning: Applies to packages/core/src/**/*.{ts,tsx} : Define styles as objects conforming to `CellStyle` type as documented in `packages/core/src/types.ts`.
Applied to files:
packages/core/src/serialization/codecs/StylesheetCodec.ts
📚 Learning: 2025-12-11T08:46:43.160Z
Learnt from: CR
Repo: maxGraph/maxGraph PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-11T08:46:43.160Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `isNullish` function when checking for `null` or `undefined` on variables that can have falsy values (numbers, strings, booleans) to avoid treating `0`, `""`, or `false` as nullish.
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-10T13:43:28.685Z
Learnt from: CR
Repo: maxGraph/maxGraph PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-10T13:43:28.685Z
Learning: Applies to packages/core/package.json : Maintain zero runtime dependencies in the core maxgraph/core package. When adding dependencies, add them only to development dependencies.
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-11T08:46:43.160Z
Learnt from: CR
Repo: maxGraph/maxGraph PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-11T08:46:43.160Z
Learning: Use `BaseGraph` for production with explicit registration of only needed features to minimize bundle size.
Applied to files:
CLAUDE.md
📚 Learning: 2025-05-13T12:54:55.231Z
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 826
File: packages/js-example-nodejs/src/index.cjs:64-69
Timestamp: 2025-05-13T12:54:55.231Z
Learning: For example code in the maxGraph repository, maintainers prefer to keep scripts simple without error handling to focus on demonstrating core functionality, especially in demonstration scripts like those in packages/js-example-nodejs.
Applied to files:
CLAUDE.md
📚 Learning: 2025-04-28T08:24:39.831Z
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 785
File: packages/core/src/view/style/register.ts:19-19
Timestamp: 2025-04-28T08:24:39.831Z
Learning: In the maxGraph project, the Perimeter namespace is defined in the 'builtin-style-elements.ts' file using the statement 'export * as Perimeter from './perimeter';'. To access the Perimeter namespace in other files, they must import from './builtin-style-elements' instead of directly from './perimeter', as part of the tree-shaking optimization.
Applied to files:
packages/core/src/view/GraphView.ts
📚 Learning: 2025-04-28T08:24:39.831Z
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 785
File: packages/core/src/view/style/register.ts:19-19
Timestamp: 2025-04-28T08:24:39.831Z
Learning: In the maxGraph project, the Perimeter namespace is defined in the './builtin-style-elements' file as a namespace export ('export * as Perimeter'), making this the correct import location for accessing the Perimeter namespace.
Applied to files:
packages/core/src/view/GraphView.ts
📚 Learning: 2025-03-25T14:13:25.331Z
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 726
File: packages/core/src/editor/EditorPopupMenu.ts:197-197
Timestamp: 2025-03-25T14:13:25.331Z
Learning: The team has decided to keep using `eval()` in EditorPopupMenu.ts as the implementation is loaded from XML configuration. Finding alternatives to `eval()` for improved security is planned for future work.
Applied to files:
packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts
🧬 Code graph analysis (10)
packages/core/src/internal/utils.ts (2)
packages/core/src/types.ts (1)
Logger(1365-1395)packages/core/src/util/config.ts (1)
GlobalConfig(45-83)
packages/core/src/view/other/PrintPreview.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
packages/core/src/serialization/codecs/StylesheetCodec.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
packages/core/src/view/layout/hierarchical/CoordinateAssignment.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
packages/core/src/gui/MaxLog.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
packages/core/src/serialization/ObjectCodec.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
packages/core/src/view/GraphView.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
packages/core/src/serialization/Codec.ts (2)
packages/core/src/internal/utils.ts (1)
log(51-51)packages/core/src/util/StringUtils.ts (1)
getFunctionName(70-93)
packages/core/src/view/plugins/ConnectionHandler.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts (1)
packages/core/src/internal/utils.ts (1)
log(51-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build (windows-2022)
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-24.04)
- GitHub Check: build
🔇 Additional comments (13)
packages/core/src/serialization/codecs/StylesheetCodec.ts (1)
24-25: LGTM:log().warn(...)swap is consistent and keeps behavior unchanged.
Nice, also keeps.jsextension compliance.Also applies to: 149-152
packages/core/src/gui/MaxLog.ts (1)
26-38: LGTM: clipboard callbacks now use the shared logger.
Noconsoleusage, and.jsimport extension is preserved.packages/core/src/internal/utils.ts (1)
64-78: LGTM:mixIntoerror path now useslog().error.packages/core/src/view/other/PrintPreview.ts (1)
29-30: LGTM:log().error(...)replacement is behavior-preserving.Also applies to: 888-891
packages/core/src/view/plugins/ConnectionHandler.ts (2)
63-64: LGTM: centralizedlog()import.
1835-1840: LGTM: error reporting still shows UI + debug message.packages/core/src/serialization/Codec.ts (1)
24-25: LGTM: warnings now route throughlog()with same messages.Also applies to: 337-338, 365-370
packages/core/src/view/GraphView.ts (1)
52-53: LGTM: enter/leave timing now useslog()and preservest0semantics.Also applies to: 544-563
CLAUDE.md (1)
249-292: Docs update is clear and matches the newlog()pattern.packages/core/src/serialization/ObjectCodec.ts (1)
26-26: Import swap tolog()looks correct (and keeps.jsextension).
No concerns with replacingGlobalConfig.loggerusage here.packages/core/src/view/layout/hierarchical/CoordinateAssignment.ts (2)
33-33:logimport path looks correct.
Keeps.jsextension per core-package rule.
204-222: Good: cacheconst logger = log()inprintStatus().
Nice pattern for multiple calls (show/info) in the same method.packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts (1)
25-26: Import migration tolog()looks good.
No concerns with replacingGlobalConfigaccess here.
|
Converted to Draft to implement coderabbitai feedback. |
|
1 participant
Add a convenient log() function in internal/utils.ts that returns GlobalConfig.logger.
This provides a shorter, more readable alternative to accessing GlobalConfig.logger directly throughout the codebase.
Also documented the pattern in CLAUDE.md for future contributors.
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.