Skip to content

Conversation

@skiyee
Copy link
Member

@skiyee skiyee commented Oct 8, 2025

resolve: #29

Summary by CodeRabbit

  • Refactor
    • Simplified detection of the project’s entry file during the build process by moving from pattern-based to explicit matching.
    • Preserves existing behavior and control flow while making the matching logic clearer and more predictable for different project setups.
    • Enhances consistency of startup handling without altering error handling or other execution paths.

@coderabbitai
Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

Updates the main-file matching in src/index.ts from a single glob pattern to an explicit array of two paths for transform-time detection. The call to registerKuApp remains triggered when the new filter matches. No other logic or exported APIs are changed.

Changes

Cohort / File(s) Summary
Main entry matching
src/index.ts
Replaces createFilter(\${rootPath}/main.(ts

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A twitch of whiskers, paths aligned,
No globbing woe to cloud my mind.
Two trails marked: .ts, .js—
I hop, I match, with gentle finesse.
Toast pops up; I thump in cheer—
The routes are clear, the root is near. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The pull request applies the explicit path filter for main.ts and main.js addressing part of [#29], but it omits the required normalization of the rootPath using normalizePath or resolve and does not add the global flag to the rootTagName regex as specified in the linked issue’s solution, so it does not fully satisfy the coding requirements in [#29]. Extend the changes to include normalizePath (or resolve) for rootPath normalization and update the rootTagName regular expression to include the global flag so all objectives of [#29] are met.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly describes the bug being fixed—issues matching paths containing parentheses—and directly relates to the main change of adjusting the file matching logic. It is concise, specific to the problem, and allows reviewers to understand the primary change without extraneous details.
Out of Scope Changes Check ✅ Passed The changes are limited to adjusting the main file matching logic in src/index.ts to use explicit paths and do not introduce any modifications unrelated to fixing the path matching failure described in [#29], so there are no out-of-scope changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/index.ts (1)

49-49: Fix platform-specific path construction.

The hardcoded backslash in \\src creates platform-specific paths that fail on Unix-based systems. While the main fix addresses parentheses in paths, this line represents another root cause mentioned in the PR objectives: "Non-normalized Windows paths and inconsistent path separators."

Apply this diff to ensure cross-platform compatibility:

-  const rootPath = process.env.UNI_INPUT_DIR || (`${process.env.INIT_CWD}\\src`)
+  const rootPath = process.env.UNI_INPUT_DIR || resolve(process.env.INIT_CWD || process.cwd(), 'src')

This ensures consistent path separator handling across all platforms.

🧹 Nitpick comments (1)
src/index.ts (1)

76-79: The explicit paths fix the parentheses issue. Consider using resolve() for full normalization.

The change from a glob pattern to explicit paths successfully avoids regex interpretation of parentheses in the path, which was the core issue. However, for consistency with lines 50 and 52, and to fully address the "normalize root path" recommendation in the PR objectives, consider using resolve() to ensure proper cross-platform path handling.

Apply this diff for complete path normalization:

-    const filterMain = createFilter([
-      `${rootPath}/main.ts`,
-      `${rootPath}/main.js`,
-    ])
+    const filterMain = createFilter([
+      resolve(rootPath, 'main.ts'),
+      resolve(rootPath, 'main.js'),
+    ])
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6aabc62 and aea4c76.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • src/index.ts (1 hunks)

@skiyee skiyee merged commit aea4c76 into uni-ku:main Oct 8, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant