Skip to content

Conversation

@ryanbas21
Copy link
Collaborator

@ryanbas21 ryanbas21 commented Dec 22, 2025

JIRA Ticket

N/A

Description

Noticed some dirs were part of the snapshot that shouldn't have been.

Summary by CodeRabbit

  • Chores
    • Updated package configuration to explicitly include distribution files in releases.
    • Reorganized dependencies, ensuring development-only tools are classified as development dependencies for a cleaner production package structure.

✏️ Tip: You can customize this high-level summary in your review settings.

@changeset-bot
Copy link

changeset-bot bot commented Dec 22, 2025

⚠️ No Changeset found

Latest commit: 624ef2f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Walkthrough

This change reorganizes dependencies in packages/journey-client/package.json by moving build and test tools (vite, vitest-canvas-mock) to devDependencies and adding a files field to explicitly include distribution artifacts in the published package.

Changes

Cohort / File(s) Summary
Dependency reorganization
packages/journey-client/package.json
Moved vite and vitest-canvas-mock from dependencies to devDependencies; retained tslib in production dependencies; added "files": ["./dist/*"] to package configuration

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • cerebrl
  • spetrov
  • ancheetah

Pre-merge checks and finishing touches

❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'chore: publish-only-dist-jc' is vague and uses abbreviated terminology that doesn't clearly convey the actual change being made to someone unfamiliar with the codebase. Clarify the title to better explain the purpose, such as 'chore: update package.json to publish only dist files for journey-client' or similar.
Description check ❓ Inconclusive The description mentions fixing snapshot directories but lacks detail on what was actually changed and why; the template's changeset section is not addressed. Expand the description to explain what directories were included incorrectly, why they should be excluded, and clarify whether a changeset was added as requested in the template.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-dist-dir-publish

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: 2

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af4bf4c and 624ef2f.

📒 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/*"],
Copy link

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.

Suggested change
"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.

Comment on lines 36 to +38
"vite": "6.4.1",
"vitest": "^1.2.0"
"vitest": "^1.2.0",
"vitest-canvas-mock": "^0.3.3"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants