Skip to content

Conversation

@skiyee
Copy link
Member

@skiyee skiyee commented Sep 13, 2025

Description 描述

ast-kit v2+ 都是esm only,会导致uni cjs项目报错

Linked Issues 关联的 Issues

Additional context 额外上下文

Summary by CodeRabbit

  • Chores
    • Pinned a core dependency to a specific 1.x version to improve stability and predictability during installs.
    • Reduces risk of unexpected behavior from future minor/patch releases of that dependency.
  • Notes
    • No user-facing changes.
    • No changes to public APIs.

@coderabbitai
Copy link

coderabbitai bot commented Sep 13, 2025

Walkthrough

Pinned the ast-kit dependency version in packages/core/package.json from ^2.1.2 to 1.4.3, narrowing the acceptable version range.

Changes

Cohort / File(s) Summary of Changes
Dependency version pinning
packages/core/package.json
Updated dependency: ast-kit from ^2.1.2 to 1.4.3 (pinned)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my nose at version strings,
From caret clouds to pinned-down things.
A tidy hop to 1.4.3,
Less wander, more certainty.
In burrows of core, I stash this kit—
Dependencies snug, a perfect fit.

Pre-merge checks and finishing touches

✅ 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 "fix(core): 修复依赖ESM-ONLY版本导致CJS项目报错问题" is concise, uses a conventional scope and type, and clearly describes the primary intent of the change (addressing an ESM-only dependency causing errors in CJS projects), which matches the changeset that pins ast-kit to a 1.x release. It therefore accurately summarizes the main change and is specific enough for a reviewer scanning PR history to understand the purpose.
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

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

🧹 Nitpick comments (2)
packages/core/package.json (2)

64-64: Use a semver range (<2) instead of an exact pin.

Pinning to "1.4.3" prevents receiving safe 1.x fixes. Prefer allowing 1.x while excluding ESM-only 2.x.

Option A (preferred, simpler):

-    "ast-kit": "1.4.3",
+    "ast-kit": "^1.4.3",

Option B (explicit ceiling):

-    "ast-kit": "1.4.3",
+    "ast-kit": ">=1.4.3 <2",

64-64: Guard transitive deps from pulling in ast-kit v2.

Add a workspace-level override to force <2 across the tree. Put this in the repo root package.json.

Example (outside this file):

{
  "overrides": {
    "ast-kit": "1.x"
  }
}

For pnpm, use pnpm.overrides if that’s your convention.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd53054 and bb995bc.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • packages/core/package.json (1 hunks)
⏰ 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). (1)
  • GitHub Check: test (windows-latest, 20)
🔇 Additional comments (1)
packages/core/package.json (1)

64-64: Add a CJS smoke test to CI (build + require CJS entry)

CI should install, build, and require packages/core/dist/index.cjs so the job fails fast if an ESM-only dep (e.g. ast-kit v2+) is introduced.

# CI / local smoke check
pnpm install --frozen-lockfile
pnpm -w -r build
node -e "require('./packages/core/dist/index.cjs'); console.log('CJS OK')"
pnpm why ast-kit   # verify resolved version is <2

Sandbox verification here failed: rimraf not found / node_modules missing — run the above in CI or locally and paste the output.

@skiyee skiyee merged commit 592ef63 into uni-helper:main Sep 13, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant