Skip to content

chore: use stricter options with the TS compiler #922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tbouffard merged 3 commits into from
Sep 24, 2025
Merged

chore: use stricter options with the TS compiler #922

tbouffard merged 3 commits into from
Sep 24, 2025

Conversation

@tbouffard
Copy link
Member

@tbouffard tbouffard commented Sep 24, 2025 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Refactor

    • Clarified logging behavior: logging entry points now explicitly return undefined when tracing is off; behavior and public APIs unchanged.

  • Chores

    • Enabled stricter TypeScript return checks (noImplicitReturns) to improve type safety and maintainability. No runtime or API impact.

@tbouffard tbouffard added the chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...) label Sep 24, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 24, 2025 •
edited
Loading

Walkthrough

Adds explicit return undefined in non-trace paths for two logging-enter methods and enables noImplicitReturns in tsconfig; no public APIs or signatures changed.

Changes

Cohort / File(s) Summary
GUI Trace Entry Handling
packages/core/src/gui/MaxLog.ts		 enter now explicitly returns undefined when TRACE is disabled; otherwise returns timestamp as before. No logic or API changes.
Logger Trace Entry Handling
packages/core/src/util/logger.ts		 ConsoleLogger.enter adds explicit return undefined in the non-trace path. No behavioral or signature changes.
TypeScript Compiler Strictness
tsconfig.json		 Adds noImplicitReturns: true to compilerOptions. No runtime or API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

javascript, skip-changelog

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request contains no author-provided description and does not follow the repository's PR template: the required checklist and the Overview sections are missing, so reviewers lack context about why the stricter TS options were added, whether there are related issues, and what tests or manual verification were performed. Please populate the repository PR template: complete the PR Checklist, add an Overview that explains the motivation for enabling stricter TypeScript options and the concrete changes (e.g., tsconfig changes and small code fixes), link any related issue(s), and note tests or manual verification steps (or why none are needed); this information will make the reviewable scope and risks clear to maintainers.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "chore: use stricter options with the TS compiler" is concise, follows Conventional Commits style, and accurately summarizes the primary change (enabling stricter TypeScript compiler options such as noImplicitReturns) shown in the diff summaries, so it communicates the main intent to reviewers scanning history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/TS_more_strict_options
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5895ab and 4116dbd.

📒 Files selected for processing (1)
  • tsconfig.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tsconfig.json

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tbouffard tbouffard marked this pull request as ready for review September 24, 2025 10:16
coderabbitai[bot]
coderabbitai bot reviewed Sep 24, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
tsconfig.json (1)

11-14: Avoid redundant flags: noImplicitAny and noImplicitThis are already implied by "strict": true

Keeping them is harmless but duplicate. noImplicitReturns is the only new check here. If you prefer a leaner config, remove the redundancies.

-    "noImplicitAny": true,
     "noImplicitOverride": true,
     "noImplicitReturns": true,
-    "noImplicitThis": true,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01451b9 and d5895ab.

📒 Files selected for processing (3)
  • packages/core/src/gui/MaxLog.ts (1 hunks)
  • packages/core/src/util/logger.ts (1 hunks)
  • tsconfig.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings 
Learnt from: tbouffard
PR: maxGraph/maxGraph#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.
🔇 Additional comments (2)
packages/core/src/util/logger.ts (1)

62-68: Explicit undefined return to satisfy noImplicitReturns — LGTM

Matches the declared signature and keeps control flow clear when tracing is disabled.

packages/core/src/gui/MaxLog.ts (1)

271-277: Consistent explicit undefined return — LGTM

Aligns with ConsoleLogger.enter and satisfies noImplicitReturns without changing behavior.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 24, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@tbouffard tbouffard merged commit f972cc9 into main Sep 24, 2025
2 checks passed
@tbouffard tbouffard deleted the chore/TS_more_strict_options branch September 24, 2025 10:27
@coderabbitai coderabbitai bot mentioned this pull request Oct 30, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tbouffard