WIP: feat: upgrade create-dev-release flow#1164
WIP: feat: upgrade create-dev-release flow#1164jwartofsky-yext wants to merge 2 commits intomainfrom
Conversation
Updates the create-dev-release flow to update the package versions in the starter to match the visual-editor To be combined with a receiving update in pages-visual-editor-starter
auto-screenshot-update: true
WalkthroughThe changes add a new capability to align shared package versions across repositories during dev release creation. A new script 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/create-dev-release.yml (1)
39-63:⚠️ Potential issue | 🟠 MajorEnsure companion changes in
pages-visual-editor-starterare ready before/alongside this PR.The two original concerns:
./starter/package.jsonavailability — This is confirmed safe. The workflow explicitly checks out the PR code and runspnpm i, which sets up thestarterworkspace (declared inpnpm-workspace.yaml). ThereadFilecall ingetSharedPackageVersions.mjswill throw if either package.json is missing, causing the step to fail clearly rather than silently producing invalid JSON.Companion changes — The
shared_package_versionsfield is new to the dispatch payload. The receiver (pages-visual-editor-starter) must be updated to handle this field before (or simultaneously with) this PR lands, or it will ignore or error on the unexpected payload.The script output format (
shared_package_versions={...}) always produces valid JSON when it succeeds, so the interpolation concern is mitigated by the fact that script errors surface as step failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/create-dev-release.yml around lines 39 - 63, This change adds a new client-payload field shared_package_versions (from steps.build-shared-package-versions.outputs.shared_package_versions) to the repository-dispatch for event-type dev-release-created targeting YextSolutions/pages-visual-editor-starter; ensure the companion repo is updated to accept and process this new payload key (or tolerate its absence) before merging: update the pages-visual-editor-starter dispatch handler to parse and validate shared_package_versions, add defensive checks for missing/invalid JSON, or make this workflow conditionally include the field only when build-shared-package-versions succeeds so the receiver remains backward-compatible.
🧹 Nitpick comments (2)
.github/workflows/README.md (1)
48-56: Docs accurately reflect the new payload.The example payload and the trailing sentence correctly describe the new
shared_package_versionsbehavior. One minor nit: consider also documenting that@yext/visual-editoritself is intentionally excluded from this map (since it's carried bypackage_url), so future readers understand why it won't appear here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/README.md around lines 48 - 56, Add a short note to the README explaining that the package named in "package_url" (e.g. "@yext/visual-editor") is intentionally excluded from the "shared_package_versions" map, so readers understand why "@yext/visual-editor" will not appear in "shared_package_versions"; update the paragraph after the example payload to mention this exclusion and clarify that "shared_package_versions" only lists other shared package names and versions.scripts/getSharedPackageVersions.mjs (1)
77-100: Consider writing only to stdout thekey=valueline and nothing else.The workflow relies on
>> "$GITHUB_OUTPUT"capturing every stdout line as akey=valuepair. Right now the script is clean, but any futureconsole.logadded for debugging (or an incidental log from an imported module) would corrupt$GITHUB_OUTPUTand likely cause the repository-dispatch payload to become invalid JSON. Consider either (a) adding a short comment here warning contributors not toconsole.logother content, or (b) writing the single output line viaprocess.stdout.writeand routing any future diagnostics toconsole.error/stderr.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/getSharedPackageVersions.mjs` around lines 77 - 100, The script's stdout must only emit the single key=value line to avoid corrupting GitHub Actions $GITHUB_OUTPUT; replace the final console.log in main() that writes shared_package_versions with a single call to process.stdout.write and ensure any diagnostics use console.error (or process.stderr.write) instead, and optionally add a short inline comment near main/await main() warning contributors not to use console.log for other output so future debug prints don't break the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/getSharedPackageVersions.mjs`:
- Around line 17-34: The spread order in getSourceDependencyLookup is backwards:
later spreads overwrite earlier ones so devDependencies currently win over
peerDependencies and dependencies; reverse the merge order so that
packageManifest.devDependencies is spread first, then
packageManifest.peerDependencies, and finally packageManifest.dependencies (or
use Object.assign({}, dev, peer, deps)) so runtime dependencies in
packageManifest.dependencies take precedence when keys conflict.
---
Outside diff comments:
In @.github/workflows/create-dev-release.yml:
- Around line 39-63: This change adds a new client-payload field
shared_package_versions (from
steps.build-shared-package-versions.outputs.shared_package_versions) to the
repository-dispatch for event-type dev-release-created targeting
YextSolutions/pages-visual-editor-starter; ensure the companion repo is updated
to accept and process this new payload key (or tolerate its absence) before
merging: update the pages-visual-editor-starter dispatch handler to parse and
validate shared_package_versions, add defensive checks for missing/invalid JSON,
or make this workflow conditionally include the field only when
build-shared-package-versions succeeds so the receiver remains
backward-compatible.
---
Nitpick comments:
In @.github/workflows/README.md:
- Around line 48-56: Add a short note to the README explaining that the package
named in "package_url" (e.g. "@yext/visual-editor") is intentionally excluded
from the "shared_package_versions" map, so readers understand why
"@yext/visual-editor" will not appear in "shared_package_versions"; update the
paragraph after the example payload to mention this exclusion and clarify that
"shared_package_versions" only lists other shared package names and versions.
In `@scripts/getSharedPackageVersions.mjs`:
- Around line 77-100: The script's stdout must only emit the single key=value
line to avoid corrupting GitHub Actions $GITHUB_OUTPUT; replace the final
console.log in main() that writes shared_package_versions with a single call to
process.stdout.write and ensure any diagnostics use console.error (or
process.stderr.write) instead, and optionally add a short inline comment near
main/await main() warning contributors not to use console.log for other output
so future debug prints don't break the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8df4c5ee-e4c9-41cb-8274-377beb8d6e39
📒 Files selected for processing (3)
.github/workflows/README.md.github/workflows/create-dev-release.ymlscripts/getSharedPackageVersions.mjs
| /** | ||
| * Builds a lookup of every dependency version the visual-editor package can source from. | ||
| * | ||
| * The order is intentional: | ||
| * 1. Runtime dependencies are used when present. | ||
| * 2. Peer dependencies can also supply versions for starter runtime dependencies. | ||
| * 3. Dev dependencies fill in shared tooling versions like TypeScript or Vite. | ||
| * | ||
| * @param {PackageManifest} packageManifest | ||
| * @returns {DependencyMap} | ||
| */ | ||
| function getSourceDependencyLookup(packageManifest) { | ||
| return { | ||
| ...packageManifest.dependencies, | ||
| ...packageManifest.peerDependencies, | ||
| ...packageManifest.devDependencies, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Spread order contradicts the documented precedence.
The JSDoc states the intended precedence is (1) dependencies, (2) peerDependencies, (3) devDependencies — i.e., runtime deps should win when a package is present in multiple sections. But because later spreads overwrite earlier keys, the current code gives devDependencies the highest precedence and dependencies the lowest. If any package (e.g., typescript, vite, a peer dep that's also pinned in devDeps for local builds) appears in more than one section of packages/visual-editor/package.json with different version strings, the starter will be aligned to the devDep version rather than the runtime one.
🔧 Proposed fix
function getSourceDependencyLookup(packageManifest) {
return {
- ...packageManifest.dependencies,
- ...packageManifest.peerDependencies,
...packageManifest.devDependencies,
+ ...packageManifest.peerDependencies,
+ ...packageManifest.dependencies,
};
}📝 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.
| /** | |
| * Builds a lookup of every dependency version the visual-editor package can source from. | |
| * | |
| * The order is intentional: | |
| * 1. Runtime dependencies are used when present. | |
| * 2. Peer dependencies can also supply versions for starter runtime dependencies. | |
| * 3. Dev dependencies fill in shared tooling versions like TypeScript or Vite. | |
| * | |
| * @param {PackageManifest} packageManifest | |
| * @returns {DependencyMap} | |
| */ | |
| function getSourceDependencyLookup(packageManifest) { | |
| return { | |
| ...packageManifest.dependencies, | |
| ...packageManifest.peerDependencies, | |
| ...packageManifest.devDependencies, | |
| }; | |
| } | |
| /** | |
| * Builds a lookup of every dependency version the visual-editor package can source from. | |
| * | |
| * The order is intentional: | |
| * 1. Runtime dependencies are used when present. | |
| * 2. Peer dependencies can also supply versions for starter runtime dependencies. | |
| * 3. Dev dependencies fill in shared tooling versions like TypeScript or Vite. | |
| * | |
| * `@param` {PackageManifest} packageManifest | |
| * `@returns` {DependencyMap} | |
| */ | |
| function getSourceDependencyLookup(packageManifest) { | |
| return { | |
| ...packageManifest.devDependencies, | |
| ...packageManifest.peerDependencies, | |
| ...packageManifest.dependencies, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/getSharedPackageVersions.mjs` around lines 17 - 34, The spread order
in getSourceDependencyLookup is backwards: later spreads overwrite earlier ones
so devDependencies currently win over peerDependencies and dependencies; reverse
the merge order so that packageManifest.devDependencies is spread first, then
packageManifest.peerDependencies, and finally packageManifest.dependencies (or
use Object.assign({}, dev, peer, deps)) so runtime dependencies in
packageManifest.dependencies take precedence when keys conflict.
1 participant
Updates the create-dev-release flow to update the package versions in the starter to match the visual-editor
To be combined with a receiving update in pages-visual-editor-starter