-
Notifications
You must be signed in to change notification settings - Fork 3
chore: publish-only-dist-jc #509
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
WalkthroughThis change reorganizes dependencies in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/journey-client/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-22T15:14:00.044Z
Learnt from: ryanbas21
Repo: ForgeRock/ping-javascript-sdk PR: 430
File: packages/journey-client/src/lib/callbacks/name-callback.ts:9-15
Timestamp: 2025-10-22T15:14:00.044Z
Learning: In packages/journey-client, callback classes are internal implementation details not part of the public API. The callbacks barrel (src/lib/callbacks/index.ts) intentionally only exports the base JourneyCallback class. Internal code imports concrete callback classes directly from their individual files (e.g., factory.ts, journey-client.ts).
Applied to files:
packages/journey-client/package.json
| "main": "./dist/src/index.js", | ||
| "module": "./dist/src/index.js", | ||
| "types": "./dist/src/index.d.ts", | ||
| "files": ["./dist/*"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Correct the files pattern and consider including documentation files.
The pattern "./dist/*" is non-standard for npm's files field:
- The leading
./is unnecessary and non-idiomatic - The pattern
dist/*may not recursively include subdirectories as intended
Use "dist" (which recursively includes all files in the dist directory) or "dist/**" for clarity. Additionally, consider including standard files like README.md and LICENSE if they exist in the package root.
🔎 Recommended fix
- "files": ["./dist/*"],
+ "files": [
+ "dist",
+ "README.md",
+ "LICENSE"
+ ],Note: package.json is included by default and doesn't need to be listed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "files": ["./dist/*"], | |
| "files": [ | |
| "dist", | |
| "README.md", | |
| "LICENSE" | |
| ], |
🤖 Prompt for AI Agents
In packages/journey-client/package.json around line 18, the "files" entry uses a
non-idiomatic and non-recursive pattern ("./dist/*"); remove the leading "./"
and replace the value with either "dist" (to include the whole directory
recursively) or "dist/**" if you prefer explicit globbing, and optionally add
"README.md" and "LICENSE" to the array so docs and license are published with
the package.
| "vite": "6.4.1", | ||
| "vitest": "^1.2.0" | ||
| "vitest": "^1.2.0", | ||
| "vitest-canvas-mock": "^0.3.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade dependencies to current versions — vite 6.4.1 and vitest ^1.2.0 are significantly outdated.
The current stable vite version is 7.2, and the latest vitest version is 4.0.15 (as of December 2025). Your pinned versions are critically behind:
- Vite 7 requires Vitest 3.2+ for testing environments, but you have vitest ^1.2.0
- Vitest 1.2.0 predates Vitest 3 and 4 releases and lacks compatibility with current Vite versions
- For vitest-canvas-mock, the latest version is 1.1.3 with active maintenance showing at least one new version released in the past 3 months, not 0.3.3
Also address the inconsistency: vite is pinned to 6.4.1 (exact version) while vitest and vitest-canvas-mock use caret ranges. Adopt a consistent versioning strategy across all dev dependencies.
🤖 Prompt for AI Agents
packages/journey-client/package.json lines 36-38: dev dependencies are pinned to
old/inconsistent versions; update "vite" to "^7.2.0", "vitest" to "^4.0.15", and
"vitest-canvas-mock" to "^1.1.3" (or to the current latest stable), and make
versioning consistent (use caret ranges for all devDependencies), then run
npm/yarn install and adjust vitest/Vite config if required for breaking changes
(e.g., update test environment/config keys to match Vitest 4 and Vite 7
compatibility).
JIRA Ticket
N/A
Description
Noticed some dirs were part of the snapshot that shouldn't have been.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.