-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
ci: sync config between projects #6130
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
Conversation
|
WalkthroughMonorepo configuration updates: changesets schema bump with an experimental peer-dep option, workspace and pnpm flags/globs expanded, CI PR workflow adds a provenance job, npm/nx/package.json adjustments, and .gitattributes LF normalization. Changes
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touchesβ Passed checks (3 passed)
β¨ Finishing touchesπ§ͺ Generate unit tests (beta)
π Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro π Files selected for processing (9)
β Files skipped from review due to trivial changes (9)
Comment |
|
View your CI Pipeline Execution β for commit 83878e0
βοΈ Nx Cloud last updated this comment at |
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: 1
π§Ή Nitpick comments (1)
.github/workflows/pr.yml (1)
56-67: Based on my verification:danielroe/provenance-action is a maintained, MIT-licensed GitHub Action that validates lockfile provenance across npm, yarn, pnpm, and bun. The action is actively documented and functional, so the availability concern in the original review is resolved.
Regarding the best-practice suggestion about SHA-pinning: The research confirms that pinning to full commit SHAs is indeed more secure than version tags (since tags can be moved by maintainers), but using semantic version tags like
v0.1.1is still acceptableβespecially when paired with organizational policies and automated update processes. This is a valid optional improvement rather than a blocker.
Use the specific version tag
v0.1.1ofdanielroe/provenance-actionor pin to the full commit SHA for enhanced supply-chain security.The provenance job adds a valuable security check for lockfile integrity. The action is maintained and available. While version tags are acceptable, pinning to the full commit SHA provides stronger assurance against tag manipulation, aligning with SLSA and GitHub's security hardening guidance. If your organization already enforces SHA-pinning or uses Dependabot for automated updates, the current version tag is sufficient.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
β Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
π Files selected for processing (7)
.changeset/config.json(2 hunks).gitattributes(1 hunks).github/workflows/pr.yml(1 hunks).npmrc(0 hunks)nx.json(2 hunks)package.json(2 hunks)pnpm-workspace.yaml(1 hunks)
π€ Files with no reviewable changes (1)
- .npmrc
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
π Additional comments (11)
.gitattributes (1)
2-2: LGTM!Explicit LF normalization ensures consistent line endings across the monorepo for all platforms. This is a common and recommended practice.
pnpm-workspace.yaml (2)
6-14: LGTM!The expanded package globs appropriately include additional example directories (lit, qwik, react, solid, svelte, vanilla, vue), aligning with the broader workspace consolidation evident across this PR.
1-3: All three workspace configuration flags are supported in pnpm 10.24.0.The flags
cleanupUnusedCatalogs,linkWorkspacePackages, andpreferWorkspacePackagesare all documented as valid workspace configuration options in pnpm 10.24.0. Setting them totruealigns with the workspace consolidation goals of this change..changeset/config.json (2)
14-16: Ensure team is aware of experimental peer-dependency behavior change.The experimental option
onlyUpdatePeerDependentsWhenOutOfRange: truechanges how changesets handles peer dependency updates. This requires coordination with the release process and should be documented for maintainers.Confirm that the team understands the implications of this experimental option for the release workflow and patch version management.
2-2: LGTM!Schema version bump to 3.1.2 is a minor version update and appropriately scoped with the experimental option change.
nx.json (2)
45-56: LGTM!The test target refactoring appropriately scopes inputs:
test:docsis focused on docs files only, enabling better cache isolationtest:knipwatches all workspace files (correct for unused code detection)test:sherifwatches onlypackage.jsonfiles (correct for dependency analysis)These changes align well with the test orchestration updates in
package.json.
4-4: Confirm nx-cloud package is available for Nx 19.3.2 support.The migration from
nxCloudAccessTokentonxCloudIdis correct and aligns with modern Nx Cloud practices. Authentication viaNX_CLOUD_ACCESS_TOKENenvironment variable (line 15 of pr.yml) is the proper CI mechanism.However, for Nx 19.3.2 (which is < 19.7), the
nx-cloudnpm package may be required for fullnxCloudIdsupport. Verify that:
- The
nx-cloudpackage is installed as a dev dependency (required for Nx < 19.7)- Developers are aware they need to run
npx nx loginto set up local personal access tokens (stored innxcloud.ini)- The workspace ID
6435ee4cd7387c45c0e4bf4cis correct for your Nx Cloud workspaceConsider documenting these setup steps for team members.
package.json (4)
4-8: LGTM!Repository format standardization and pnpm version bump are well-motivated:
- Object format for repository metadata follows npm best practices
- pnpm 10.16.1 β 10.24.0 is a minor version bump, safe and aligns with workspace configuration updates
13-14: LGTM!Test script orchestration is well-coordinated:
test:prandtest:cinow includetest:docstarget- New
test:docsscript delegates toscripts/verify-links.tsnx.includedScriptsupdated to includetest:docs- Changes align with nx.json target additions and workflow intentions
Verify that
scripts/verify-links.tsexists and is functional. If this is a new file, provide context on what it does.Also applies to: 21-21, 34-34
49-50: The removals are intentional and safeβno issues found.The
@tanstack/configdevDependency andpreinstallscript removals don't pose any problems:
@tanstack/configremoval is safe: The GitHub workflow actiontanstack/config/.github/setup@main(referenced in pr.yml lines 32 & 51) is independent of the npm package. The action is publicly accessible and handles all CI setup needs (pnpm, Node, caching, dependency installation). The npm package was likely for sharing internal configuration files, not required for the table project's own build.
preinstallscript removal is confirmed: No references to it exist in the codebase, indicating it was safely removed with no lingering dependencies.Workflow remains functional: The setup action continues to work correctly (verified HTTP 200 + active maintenance) and will properly initialize the environment for PR checks.
78-78: Based on my verification, I have gathered sufficient information to rewrite the review comment. Here's what I found:Key findings:
- v1.0.0 Release Notes: The release introduces new features (unordered-dependencies rule, auto-install on fix, nested workspace packages support) but does NOT list any breaking changes to the CLI interface.
- CLI Compatibility: The
-iflag used intest:sherif(for--ignore-dependency) is explicitly documented and supported in v1.0.0+.- Current Installation: The project currently has sherif v1.9.0 installed (resolves from
^1.6.1).- Command Syntax: The test script
sherif -i react -i react-dom -i vue -i solid-js -i svelte -i @builder.io/qwikmatches the documented v1.0.0+ CLI interface exactly.
Sherif v1.6.1 is backward compatible with existing usage.
The version bump from v0.9.0 to v1.6.1 introduces new features (unordered-dependencies rule, automatic package manager install on
--fix, nested workspace packages) but does not change the CLI interface. The-iflag for ignoring dependencies remains supported and thetest:sherifscript is fully compatible.
| "watch": "pnpm run build:all && nx watch --all -- pnpm run build:all", | ||
| "dev": "pnpm run watch", | ||
| "prettier": "prettier --ignore-unknown '**/*'", | ||
| "prettier": "prettier --experimental-cli --ignore-unknown '**/*'", |
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.
π§© Analysis chain
π Web query:
prettier 3.3.2 --experimental-cli flag documentation
π‘ Result:
Short answer: The --experimental-cli flag is not part of Prettier 3.3.2 β it was introduced later as an experimental high-performance CLI behind a feature flag (usable as --experimental-cli or via PRETTIER_EXPERIMENTAL_CLI=1) and documented in Prettier release/blog and CLI docs. See Prettier blog announcing the experimental CLI and the CLI docs for usage. [1][2][3]
Sources
- Prettier release/blog announcing the experimental CLI (June 23, 2025). [1]
- Prettier CLI documentation (flags and CLI usage). [2]
- Community/announcement explaining how to enable it (example usage). [3]
π Script executed:
cat package.json | head -30Repository: TanStack/table
Length of output: 1560
π Script executed:
rg -A 50 '"devDependencies"' package.json | head -80Repository: TanStack/table
Length of output: 1248
The --experimental-cli flag is not available in prettier 3.3.2 and will cause an error.
According to prettier documentation, the --experimental-cli flag was introduced after version 3.3.2 (announced June 23, 2025). Since the package.json specifies "prettier": "^3.3.2", using this flag will fail if prettier 3.3.2 is installed. Either remove the flag or update the prettier version constraint to a version that supports it (3.4.0 or later, if available, or remove the caret to allow installation of newer versions).
π― Changes
@tanstack/config(not used)β Checklist
pnpm test:pr.π Release Impact
Summary by CodeRabbit
βοΈ Tip: You can customize this high-level summary in your review settings.