Chore: Upgrade Node.js, improve publishing scripts, and fix Angular tests#1177
Chore: Upgrade Node.js, improve publishing scripts, and fix Angular tests#1177jacobsimionato wants to merge 4 commits intogoogle:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the package preparation and publishing scripts to dynamically resolve internal @a2ui/ dependencies via a workspace graph and extends publishing safety checks to include @a2ui/markdown-it. Additionally, it updates Angular component tests to verify styles on the host element and refines the web_core test execution command. Review feedback recommends using more idiomatic JavaScript for dependency updates, including optionalDependencies in the process, and dynamically identifying packages for safety checks to eliminate hardcoded lists.
| const updateInternalDeps = (deps) => { | ||
| if (!deps) return; | ||
| for (const name in deps) { | ||
| const version = deps[name]; | ||
| if (version.startsWith('file:') && graph[name]) { | ||
| deps[name] = '^' + graph[name].version; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Refactoring updateInternalDeps to use Object.entries is cleaner and more idiomatic in modern JavaScript. Additionally, throwing an error when a file: dependency cannot be resolved in the workspace graph prevents the accidental publication of broken packages that contain unresolved local links.
| const updateInternalDeps = (deps) => { | |
| if (!deps) return; | |
| for (const name in deps) { | |
| const version = deps[name]; | |
| if (version.startsWith('file:') && graph[name]) { | |
| deps[name] = '^' + graph[name].version; | |
| } | |
| } | |
| }; | |
| const updateInternalDeps = (deps) => { | |
| if (!deps) return; | |
| for (const [name, version] of Object.entries(deps)) { | |
| if (typeof version === 'string' && version.startsWith('file:')) { | |
| if (graph[name]) { | |
| deps[name] = '^' + graph[name].version; | |
| } else { | |
| throw new Error(`Internal dependency ${name} not found in workspace.`); | |
| } | |
| } | |
| } | |
| }; |
| updateInternalDeps(pkg.dependencies); | ||
| updateInternalDeps(pkg.peerDependencies); |
There was a problem hiding this comment.
The script should also update internal dependencies in optionalDependencies to ensure they are correctly resolved when the package is published to npm. Currently, only dependencies and peerDependencies are handled.
| updateInternalDeps(pkg.dependencies); | |
| updateInternalDeps(pkg.peerDependencies); | |
| updateInternalDeps(pkg.dependencies); | |
| updateInternalDeps(pkg.peerDependencies); | |
| updateInternalDeps(pkg.optionalDependencies); |
| const renderers = ['@a2ui/lit', '@a2ui/angular', '@a2ui/react']; | ||
| const requestedRenderers = resolvedPackages.filter(p => renderers.includes(p)); |
There was a problem hiding this comment.
Instead of hardcoding the list of renderers, you can dynamically identify packages that depend on core libraries (like @a2ui/web_core or @a2ui/markdown-it) using the package graph. This makes the safety check more robust as new packages are added to the mono-repo.
const corePackages = [webCoreName, markdownItName];
const requestedRenderers = resolvedPackages.filter(name => {
const pkg = graph[name];
return pkg && pkg.internalDependencies.some(dep => corePackages.includes(dep));
});
Description of Changes
This PR introduces several improvements to the build, test, and publishing infrastructure:
20to22across all GitHub Action workflows.prepare-publish.mjsto dynamically update all internal@a2ui/dependencies (usingfile:links) to their current workspace versions. This replaces the previous hardcoded logic that only handled@a2ui/web_core.publish_npm.mjssafety checks to ensure@a2ui/markdown-itis also published alongside renderers, preventing version mismatches for shared dependencies.publish_npm.test.mjsto verify the new topological sorting and publishing flow includingmarkdown-it.ColumnComponentandRowComponentunit tests to correctly verify styles on the host element and updated expected flex values to align with standard CSS (e.g.,flex-start).@a2ui/web_coreto correctly target compiled test files in thedist/directory.Rationale
Testing/Running Instructions
cd renderers/angular npm install npm run test:ciThe integration tests for the publishing scripts can be run via:
cd renderers node --test scripts/publish_npm.test.mjs