Skip to content

fix(sdk): restore package.json copy for Vite SDK publish after nxViteTsPaths removal - #36598

Merged
nicobytes merged 2 commits into
mainfrom
36540-fix
Jul 15, 2026
Merged

fix(sdk): restore package.json copy for Vite SDK publish after nxViteTsPaths removal#36598
nicobytes merged 2 commits into
mainfrom
36540-fix

Conversation

@nicobytes

@nicobytes nicobytes commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Restores explicit package.json (and README) copy into dist for @dotcms/analytics and @dotcms/vue via viteStaticCopy, replacing a hidden side effect of the removed nxViteTsPaths() plugin.
  • Hardens the SDK NPM publish action to skip dist directories that have no package.json (e.g. analytics-standalone), so the publish loop does not fail with jq: Could not open file package.json.

Context

Trunk SDK @next publish failed on run 29416418823 after #36575 replaced nxViteTsPaths() with vite-tsconfig-paths. The Nx plugin copied package.json into the Vite outDir on writeBundle; the replacement does not. Publish then crashed while processing @dotcms/analytics after partially publishing ai, angular, and types as 1.7.1-next.2363.

Test plan

  • pnpm exec nx build sdk-analytics --configuration=productiondist/libs/sdk/analytics/package.json present
  • pnpm exec nx build sdk-vue --configuration=productiondist/libs/sdk/vue/package.json present
  • After merge, next trunk deployment publishes a full coherent @next set (new run_number; do not re-run 2363)

This PR fixes: #36540

This PR fixes: #36540

…sdk action

- Added a check to skip processing for SDKs that do not contain a package.json file, preventing unnecessary errors during deployment.
- Updated vite.config.mts for both analytics and vue SDKs to include package.json in the static copy targets, ensuring it is copied to the distribution directory alongside markdown files.

This change enhances the deployment process by ensuring only valid SDKs are processed and necessary files are included in the build output.
@nicobytes
nicobytes requested a review from a team as a code owner July 15, 2026 18:01
@nicobytes nicobytes changed the title fix(deployment): fix(sdk): restore package.json copy for Vite SDK publish after nxViteTsPaths removal Jul 15, 2026
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @nicobytes's task in 5m 53s —— View job


Code Review

  • Read PR diff (both vite configs + the two action.yml commits)
  • Verify the package.json copy mechanism
  • Check the publish-action hardening claim
  • Post review

Summary

The core fix is sound. Both viteStaticCopy changes correctly restore the package.json copy that nxViteTsPaths() used to perform implicitly. src globs (*.md, package.json) resolve relative to each config's Vite root (__dirname / import.meta.dirname = the lib dir), and dest: '.' lands them at the outDir root — matching the pattern @dotcms/vue already used for *.md. The removed copyReadme fs plugin in analytics is fully subsumed by { src: '*.md', dest: '.' }, and its import fs is correctly dropped while path / react remain in use. vite-plugin-static-copy@4.1.1 is already a dependency. No bug in the two changed files.

New Issues

  • 🟡 Medium: .github/actions/core-cicd/deployment/deploy-javascript-sdk/action.yml:206 — The PR description says it "hardens the SDK NPM publish action to skip dist directories that have no package.json," but that guard was added in a9ac78b4 and then removed again in 2fc80d44, so git diff origin/main...HEAD on action.yml is empty — the committed action is identical to main. The publish loop still runs jq -r '.version' package.json unconditionally (line 206) for every dir under dist/libs/sdk/. This is safe only because analytics-standalone is produced by the build:standalone target, and the publish build runs nx run-many --target=build --projects='sdk-*' (line 179) — which does not invoke build:standalone — so dist/libs/sdk/analytics-standalone is never present in a clean CI checkout.

    If that assumption ever changes (e.g. build:standalone becomes a dependency of build, or a stale/pre-populated dist is reused), the original jq: Could not open file package.json failure returns. Either re-add the skip guard to match the stated intent, or update the PR description to drop the hardening claim so reviewers aren't relying on a safety net that isn't in the merged code. Fix this →

Not blocking — the merged behavior is correct for the current build graph. Flagging only the description/code mismatch.

@github-actions github-actions Bot added Area : CI/CD PR changes GitHub Actions/workflows Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries labels Jul 15, 2026
…t-sdk action

- Eliminated the check for the presence of package.json in the deploy-javascript-sdk action, streamlining the deployment process for SDKs.
- This change allows the action to proceed without skipping SDKs based on the absence of package.json, enhancing flexibility in deployment.

This update simplifies the deployment logic and focuses on processing SDKs without unnecessary checks.
@github-actions github-actions Bot removed the Area : CI/CD PR changes GitHub Actions/workflows label Jul 15, 2026
@nicobytes
nicobytes enabled auto-merge July 15, 2026 18:10
@nicobytes
nicobytes added this pull request to the merge queue Jul 15, 2026
@mergify

mergify Bot commented Jul 15, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Merged via the queue into main with commit 31a2323 Jul 15, 2026
42 checks passed
@nicobytes
nicobytes deleted the 36540-fix branch July 15, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Nx: Replace deprecated nxViteTsPaths and nxCopyAssetsPlugin plugin

3 participants