Switch from yarn to pnpm, improve workflows
#1863
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1863 +/- ##
=======================================
Coverage 98.83% 98.83%
=======================================
Files 18 18
Lines 1547 1547
Branches 334 334
=======================================
Hits 1529 1529
Misses 18 18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
"Tests" workflow has a lot of repeating checkouts and builds, this bothers me but it looks like there's no simple solution to this, for whatever reason. |
Maybe in the future we could optimize by using something like nx. |
|
@Strift I have seen what curquiza wrote you. While that's great for protecting others from us getting compromised, we're still not protected from others being compromised against us. https://pnpm.io/supply-chain-security Just another reason to try and convince your colleagues to switch to pnpm. |
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
Would you be up for fixing the conflicts?
I feel comfortable moving forward; i'll let the team know
|
Sure. |
|
Warning Rate limit exceeded@flevi29 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 40 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (19)
WalkthroughMigrates the repository from Yarn to pnpm: introduces a composite GitHub Action to set up Node and pnpm, converts workflows and scripts to pnpm, updates workspace/package config, docs, and test env files, and replaces yarn invocations with pnpm across CI and local tooling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/env/nitro-app/package.json (1)
1-16: Critical: Resolve lockfile mismatch before merging.The pipeline reports dependency version mismatches between the lockfile and package.json:
@typescript-eslint/utils: lockfile has^8.29.0, manifest expects^8.48.1prettier: lockfile has^3.6.2, manifest expects^3.7.4This indicates the pnpm lockfile is out of sync with dependencies. Run
pnpm installat the repository root to regenerate the lockfile and resolve the mismatch.#!/bin/bash # Description: Verify the lockfile exists and check for version mismatches in root package.json # Check if pnpm-lock.yaml exists at root if [ -f "pnpm-lock.yaml" ]; then echo "✓ pnpm-lock.yaml found at root" else echo "✗ pnpm-lock.yaml missing at root" fi # Check for typescript-eslint/utils and prettier versions in root package.json echo "Checking dependency versions in package.json:" cat package.json | jq -r '.dependencies["@typescript-eslint/utils"], .devDependencies["@typescript-eslint/utils"], .dependencies["prettier"], .devDependencies["prettier"]' | grep -v null || echo "Dependencies may be in nested packages"
🧹 Nitpick comments (3)
.github/dependabot.yml (1)
3-3: Standardize YAML quoting style for consistency.The file mixes quoted and unquoted string values, which introduces unnecessary inconsistency. For example, line 3 quotes
"github-actions"while line 11 leavesnpmunquoted; similarly, line 8 quotes the"dependencies"label while line 19 does not. Consider standardizing all string values to use consistent quoting.Apply this diff to standardize quoting:
- - package-ecosystem: npm - directory: "/" + - package-ecosystem: "npm" + directory: "/" schedule: interval: "monthly" time: "04:00" open-pull-requests-limit: 10 labels: - - dependencies + - "dependencies"Also applies to: 11-11, 8-8, 19-19
tests/search.test.ts (1)
1362-1379: Refactor abort test to await the rejected promise instead of disabling lint rulesRight now the test schedules
searchPromise.catchbut neverawaits orreturnsit, so the assertion might not be observed by Vitest, and the eslint disables just mask that issue. You can make the test reliable and drop the disables by awaiting the rejection directly:test(`${permission} key: search on index and abort`, async () => { const controller = new AbortController(); const client = await getClient(permission); const searchPromise = client .index(index.uid) .search("unreachable", {}, { signal: controller.signal }); controller.abort(); - // TODO: Fix - // eslint-disable-next-line vitest/valid-expect-in-promise - searchPromise.catch((error) => { - // eslint-disable-next-line vitest/no-conditional-expect - expect(error).toHaveProperty( - "cause.message", - "This operation was aborted", - ); - }); + await expect(searchPromise).rejects.toHaveProperty( + "cause.message", + "This operation was aborted", + ); });This keeps the behavior while satisfying the lint rules and ensuring the assertion is actually awaited.
.github/workflows/tests.yml (1)
5-6: Tests workflow is consistent with pnpm migration and composite setup actionThe integration, style, and types jobs now all rely on the shared
./.github/actions/set-up-nodeaction and pnpm-based scripts (pnpm test,pnpm build,pnpm test:env:*,pnpm style,pnpm types), which keeps CI configuration DRY and aligned withpackage.json.Minor note: style and types jobs currently use the composite action’s default
node-version(lts/*), while integration tests explicitly run on Node 20 and 22. That’s fine, but if you ever need strict consistency, you can also passnode-versioninto those jobs.Once the composite action has a
descriptionfield, the actionlint warning referenced for this workflow will also be resolved.Also applies to: 32-40, 42-59, 68-73, 81-87
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/env/esm/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/env/esm/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/express/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/env/express/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/nitro-app/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/typescript-browser/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/typescript-node/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
.code-samples.meilisearch.yaml(1 hunks).github/actions/set-up-node/action.yml(1 hunks).github/dependabot.yml(1 hunks).github/release-draft-template.yml(2 hunks).github/workflows/docs.yml(1 hunks).github/workflows/meilisearch-prototype-tests.yml(2 hunks).github/workflows/pre-release-tests.yml(2 hunks).github/workflows/publish.yml(1 hunks).github/workflows/tests.yml(4 hunks).gitignore(1 hunks).husky/pre-commit(1 hunks)CONTRIBUTING.md(3 hunks)package.json(2 hunks)playgrounds/javascript/README.md(1 hunks)pnpm-workspace.yaml(1 hunks)tests/env/browser/README.md(1 hunks)tests/env/esm/package.json(1 hunks)tests/env/nitro-app/package.json(1 hunks)tests/env/node/README.md(1 hunks)tests/env/typescript-browser/README.md(2 hunks)tests/search.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and CI workflows use the latest Meilisearch image (getmeili/meilisearch:latest) for consistency between development and testing environments.
Applied to files:
playgrounds/javascript/README.md.github/workflows/tests.ymltests/env/node/README.md.github/workflows/meilisearch-prototype-tests.ymltests/env/typescript-browser/README.mdCONTRIBUTING.mdtests/env/browser/README.md.github/workflows/pre-release-tests.yml
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and the main CI workflow (.github/workflows/tests.yml) use `getmeili/meilisearch:latest` to maintain consistency between local development and CI testing environments.
Applied to files:
playgrounds/javascript/README.md.github/workflows/tests.ymltests/env/node/README.md.github/workflows/publish.yml.github/workflows/meilisearch-prototype-tests.ymltests/env/typescript-browser/README.mdtests/env/esm/package.jsonCONTRIBUTING.mdtests/env/browser/README.md.github/workflows/pre-release-tests.yml.code-samples.meilisearch.yaml
📚 Learning: 2025-09-04T06:44:01.679Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 2012
File: tests/client.test.ts:13-13
Timestamp: 2025-09-04T06:44:01.679Z
Learning: The meilisearch-js project supports Node.js 20 and above, so JSON imports should use the modern `with { type: "json" }` syntax rather than the deprecated `assert { type: "json" }` syntax.
Applied to files:
playgrounds/javascript/README.mdCONTRIBUTING.md.code-samples.meilisearch.yaml
🪛 actionlint (1.7.9)
.github/workflows/tests.yml
44-44: description is required in metadata of "Set up Node.js and install dependencies" action at "/home/jailuser/git/.github/actions/set-up-node/action.yml"
(action)
.github/workflows/publish.yml
22-22: description is required in metadata of "Set up Node.js and install dependencies" action at "/home/jailuser/git/.github/actions/set-up-node/action.yml"
(action)
29-29: string should not be empty
(syntax-check)
29-29: unexpected end of input while parsing variable access, function call, null, bool, int, float or string. expecting "IDENT", "(", "INTEGER", "FLOAT", "STRING"
(expression)
.github/workflows/meilisearch-prototype-tests.yml
54-54: description is required in metadata of "Set up Node.js and install dependencies" action at "/home/jailuser/git/.github/actions/set-up-node/action.yml"
(action)
.github/workflows/pre-release-tests.yml
51-51: description is required in metadata of "Set up Node.js and install dependencies" action at "/home/jailuser/git/.github/actions/set-up-node/action.yml"
(action)
.github/workflows/docs.yml
16-16: description is required in metadata of "Set up Node.js and install dependencies" action at "/home/jailuser/git/.github/actions/set-up-node/action.yml"
(action)
🪛 GitHub Actions: Tests
tests/env/nitro-app/package.json
[error] 1-1: Dependency specifiers mismatch with lockfile: @typescript-eslint/utils (lockfile ^8.29.0, manifest ^8.48.1) and prettier (lockfile ^3.6.2, manifest ^3.7.4).
tests/env/esm/package.json
[error] 1-1: Dependency specifiers mismatch with lockfile: @typescript-eslint/utils (lockfile ^8.29.0, manifest ^8.48.1) and prettier (lockfile ^3.6.2, manifest ^3.7.4).
package.json
[error] 1-1: Dependency specifiers mismatch with lockfile: @typescript-eslint/utils (lockfile ^8.29.0, manifest ^8.48.1) and prettier (lockfile ^3.6.2, manifest ^3.7.4).
🔇 Additional comments (19)
.github/release-draft-template.yml (1)
19-23: Inconsistency between summary and code: documentation label status unclear.The AI summary claims that a new
documentationlabel was added to the "⚙️ Maintenance/misc" category, but line 23 is not marked as changed (~) in the provided code. Additionally, lines 21-23 appear to be unmarked while most other lines in the file show quote-style changes (single to double quotes). This discrepancy suggests either:
- The
documentationlabel was already present (not newly added), or- There may be a code display issue with the change markers.
Please clarify whether this label is newly added or pre-existing.
.husky/pre-commit (1)
1-1: LGTM! Pre-commit hook correctly updated to pnpm.The migration from yarn to pnpm for the lint-staged hook is correct.
playgrounds/javascript/README.md (1)
3-3: LGTM! Documentation correctly updated to pnpm.The installation and development commands have been properly migrated to pnpm, and the description now accurately reflects the tooling.
Also applies to: 10-10, 16-16
.gitignore (1)
135-135: LGTM! Gitignore correctly updated for pnpm lockfile.The change from ignoring yarn.lock to pnpm-lock.yaml in the playgrounds directory is appropriate for the package manager migration.
tests/env/node/README.md (1)
9-9: LGTM! Build command correctly updated to pnpm.The build instruction has been properly migrated to use pnpm.
tests/env/browser/README.md (1)
9-9: LGTM! Build command correctly updated to pnpm.The build instruction has been properly migrated to use pnpm.
tests/env/typescript-browser/README.md (1)
9-9: LGTM! All commands correctly updated to pnpm.The installation and build instructions have been properly migrated to use pnpm throughout the workflow.
Also applies to: 25-25, 31-31
tests/env/nitro-app/package.json (1)
4-4: LGTM! Test script correctly updated to pnpm.The test script has been properly migrated from yarn to pnpm.
pnpm-workspace.yaml (1)
1-2: LGTM! Configuration aligns with pnpm best practices.The
onlyBuiltDependenciessetting for esbuild is appropriate since esbuild uses native binaries. This prevents pnpm from attempting to rebuild esbuild from source.Verify that no other dependencies with native builds require similar treatment (common packages: fsevents, sharp, node-sass, canvas, sqlite3, deasync).
tests/env/esm/package.json (1)
6-9: ESM env start script matches pnpm-based workflowSwitching the
startscript to usepnpm buildis consistent with the newtest:env:esmpipeline and looks correct..code-samples.meilisearch.yaml (1)
424-430: Getting-started snippet correctly adds pnpm exampleThe added
pnpm add meilisearchsnippet complements the existing npm example and reflects the repository’s move to pnpm without breaking existing guidance.CONTRIBUTING.md (1)
37-71: Contributing workflow docs aligned with pnpm and current scriptsThe updated requirements and example commands (docker, install, tests, style, build) correctly reflect the pnpm-based workflow and existing
package.jsonscripts..github/workflows/docs.yml (1)
15-20: Docs workflow correctly reuses the composite Node/pnpm setupUsing
./.github/actions/set-up-nodehere and switching topnpm build:docsmatches the new scripts and keeps the docs job consistent with the rest of the CI. Once the composite action gains adescriptionfield, actionlint should also be happy for this workflow.package.json (1)
41-65: Lockfile sync issue requires verification—cannot determine current stateThe original review flagged a devDependency version mismatch between package.json and pnpm-lock.yaml causing CI failure. Without access to the current repository state, I cannot confirm whether the lockfile has been updated to resolve this or if the issue persists. A manual check of package.json and pnpm-lock.yaml is needed to verify the current status of @typescript-eslint/utils and prettier versions.
.github/workflows/publish.yml (3)
29-29: Verify conditional expression syntax.Actionlint is flagging the
if: !github.event.release.prereleaseexpression as having parsing issues, but the syntax is valid for GitHub Actions. Ensure this expression correctly evaluates at runtime by testing the publish workflow in CI.If this is a known actionlint limitation with negation operators, you may safely ignore the hint. Otherwise, consider rephrasing as:
if: github.event.release.prerelease == false
27-27: LGTM: Pnpm migration and provenance implementation.The migration to pnpm commands is correct, and the addition of
--provenance --access publicflags properly implements supply chain security for npm package publishing. This aligns with the PR objectives and npm best practices.Also applies to: 30-30, 35-35
22-24: Add required description field to composite action metadata.The composite action at
./.github/actions/set-up-node/action.ymlis missing a requireddescriptionfield in its action.yml metadata. GitHub Actions requires this field for composite actions.Add the missing
descriptionfield to./.github/actions/set-up-node/action.yml:name: Set up Node.js and install dependencies description: Set up Node.js with pnpm caching runs: using: 'composite' # ... rest of action configuration.github/workflows/meilisearch-prototype-tests.yml (1)
6-6: LGTM: YAML consistency and pnpm migration.The updates to use consistent double-quoted strings throughout and the comprehensive migration to pnpm commands are correctly implemented. All test environment commands (test:env:esm, test:env:nodejs, etc.) are properly updated.
Also applies to: 12-12, 15-15, 38-38, 43-44, 46-46, 49-49, 59-67
.github/workflows/pre-release-tests.yml (1)
6-6: LGTM: YAML consistency and pnpm migration.The updates to use consistent double-quoted strings and the comprehensive migration to pnpm commands are correctly implemented. All test environment commands are properly updated to use pnpm.
Also applies to: 12-13, 16-17, 35-35, 40-41, 43-43, 46-46, 56-66
flevi29
changed the title
yarn to pnpm, improve workflows, add provenance to published packageyarn to pnpm, improve workflows
|
@Strift All done! Updated description of this PR. |
|
BTW, yarn does have similar flag: If this is the only security feature we were looking at. |
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
Great work 🙌 thanks for bumping this PR and updating it promptly!
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
Strift
force-pushed
the
yarn-to-pnpm-and-workflows
branch
from
cb054b0 to
5a62624
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/env/esm/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/express/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/nitro-app/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/typescript-browser/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/typescript-node/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.github/actions/set-up-node/action.yml(1 hunks).github/dependabot.yml(1 hunks).github/release-draft-template.yml(2 hunks).github/workflows/docs.yml(1 hunks).github/workflows/meilisearch-prototype-tests.yml(2 hunks).github/workflows/pre-release-tests.yml(2 hunks).github/workflows/publish.yml(1 hunks).github/workflows/tests.yml(4 hunks)CONTRIBUTING.md(3 hunks)package.json(1 hunks)playgrounds/javascript/README.md(1 hunks)pnpm-workspace.yaml(1 hunks)tests/env/browser/README.md(1 hunks)tests/env/esm/package.json(1 hunks)tests/env/nitro-app/package.json(1 hunks)tests/env/node/README.md(1 hunks)tests/env/typescript-browser/README.md(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/env/browser/README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/env/typescript-browser/README.md
- .github/workflows/docs.yml
- tests/env/nitro-app/package.json
- playgrounds/javascript/README.md
- tests/env/esm/package.json
- pnpm-workspace.yaml
- .github/release-draft-template.yml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and the main CI workflow (.github/workflows/tests.yml) use `getmeili/meilisearch:latest` to maintain consistency between local development and CI testing environments.
Applied to files:
.github/workflows/tests.ymltests/env/node/README.md.github/workflows/meilisearch-prototype-tests.ymlCONTRIBUTING.md.github/workflows/pre-release-tests.yml
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and CI workflows use the latest Meilisearch image (getmeili/meilisearch:latest) for consistency between development and testing environments.
Applied to files:
.github/workflows/tests.ymltests/env/node/README.md.github/workflows/meilisearch-prototype-tests.ymlCONTRIBUTING.md.github/workflows/pre-release-tests.yml
📚 Learning: 2025-09-04T06:44:01.679Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 2012
File: tests/client.test.ts:13-13
Timestamp: 2025-09-04T06:44:01.679Z
Learning: The meilisearch-js project supports Node.js 20 and above, so JSON imports should use the modern `with { type: "json" }` syntax rather than the deprecated `assert { type: "json" }` syntax.
Applied to files:
CONTRIBUTING.md
🪛 actionlint (1.7.9)
.github/workflows/tests.yml
67-67: could not parse as YAML: could not find expected ':'
(syntax-check)
.github/workflows/publish.yml
29-29: could not parse as YAML: could not find expected ':'
(syntax-check)
🔇 Additional comments (13)
tests/env/node/README.md (1)
9-9: LGTM!Documentation update correctly reflects the pnpm migration.
.github/dependabot.yml (1)
11-24: VerifyminimumReleaseAgesetting per PR objectives.The PR description mentions "protection against updated packages via pnpm's minimumReleaseAge setting," but this configuration is not visible in the npm ecosystem block. Confirm whether this setting should be added to dependabot.yml to align with the stated security objective.
CONTRIBUTING.md (1)
51-51: ✅ pnpm commands correctly replace Yarn.All command references have been properly updated from Yarn to pnpm equivalents (pnpm install, pnpm test, pnpm style, pnpm build, etc.). Once the merge conflict above is resolved, these updates look correct.
Also applies to: 57-57, 68-68, 70-70, 72-72, 74-74
package.json (3)
100-100: ✅ Package manager correctly specified.The
packageManagerfield correctly declares [email protected], which enforces usage across the repository and integrates with GitHub Actions caching.
48-63: ✅ pnpm scripts properly structured.Scripts correctly use
pnpmcommands and-C(cwd) flags for workspace package operations (e.g.,pnpm -C tests/env/express test). This aligns with pnpm workspace best practices.
70-79: ✅ lint-staged configuration added.The new lint-staged configuration properly integrates prettier formatting and eslint fixes as pre-commit hooks. This is a good addition for maintaining code quality without additional tooling.
.github/actions/set-up-node/action.yml (1)
1-21: ✅ Composite action correctly structured with pnpm setup.The action properly:
- Includes required metadata (
nameanddescriptionfields)- Installs pnpm via the official action
- Configures Node.js with
cache: pnpmfor CI optimization- Runs
pnpm installas the final setup stepThis centralizes Node.js setup across workflows and reduces duplication.
.github/workflows/meilisearch-prototype-tests.yml (1)
38-68: ✅ Workflow correctly migrated to pnpm and local setup action.Updates are consistent across the entire workflow:
- Matrix key properly changed from
nodetonode-versionwith expanded versions ["18", "20", "22"]- Job name correctly updated to reference
matrix.node-version- Local
./.github/actions/set-up-nodeaction properly integrated- All environment commands updated to pnpm equivalents
Based on learnings, the Docker image consistency is maintained appropriately for prototype testing.
.github/workflows/tests.yml (1)
42-58: ✅ integration_tests job correctly configured.The integration_tests job (lines 42-58) is properly resolved:
- Correct use of local setup action
- Matrix key correctly changed from
nodetonode-version- All pnpm commands correctly in place
- No merge conflicts
.github/workflows/pre-release-tests.yml (4)
49-52: Verify checkout action downgrade from v6 to v4.The checkout action was downgraded from
v6tov4(line 49). This appears intentional as part of the workflow update, but it's unclear why a downgrade was made. Please confirm this is not an unintended regression.
23-24: LGTM: Job output and dependency chain is properly configured.The
meilisearch-versionjob correctly exports theversionoutput, and theintegration_testsjob properly declaresneeds: ["meilisearch-version"]to consume it. This enables the service image on line 38 to use the correct pre-release version.Also applies to: 35-35
45-47: LGTM: Matrix variable renamed for clarity.The matrix variable is renamed from
nodetonode-version, which improves readability. The job name on line 47 correctly referencesmatrix.node-version.
55-55: LGTM: pnpm commands correctly replace yarn equivalents.All package manager commands have been properly migrated from
yarntopnpm. The test and build steps align with the package manager migration objectives.Also applies to: 57-57, 59-59, 61-61, 63-63, 65-65
|
@Strift I'm going to try and rebase myself, taking my time with it. It is troublesome, it's hard and annoying. |
|
@flevi29, just saw your comment. I think I managed to fix the conflicts, but now the tests are failing 😅 I'm happy to take this over the finish line if necessary. Let me know. Let's keep in mind the PR needs to be rebase-able for the merge queue requirements |
flevi29
force-pushed
the
yarn-to-pnpm-and-workflows
branch
from
edc2dde to
2c1b6bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/release-draft-template.yml (1)
1-32: Quote style standardization—no functional impact.YAML is valid with both single and double quotes. This standardization improves consistency across configuration files without affecting release draft generation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/env/esm/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/express/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/nitro-app/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/typescript-browser/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/env/typescript-node/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.github/actions/set-up-node/action.yml(1 hunks).github/dependabot.yml(1 hunks).github/release-draft-template.yml(1 hunks).github/workflows/docs.yml(1 hunks).github/workflows/meilisearch-prototype-tests.yml(2 hunks).github/workflows/pre-release-tests.yml(2 hunks).github/workflows/publish.yml(1 hunks).github/workflows/tests.yml(3 hunks)CONTRIBUTING.md(2 hunks)package.json(2 hunks)playgrounds/javascript/README.md(1 hunks)pnpm-workspace.yaml(1 hunks)tests/env/browser/README.md(1 hunks)tests/env/esm/package.json(1 hunks)tests/env/nitro-app/package.json(1 hunks)tests/env/node/README.md(1 hunks)tests/env/typescript-browser/README.md(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- pnpm-workspace.yaml
- CONTRIBUTING.md
- tests/env/typescript-browser/README.md
- .github/workflows/docs.yml
- tests/env/nitro-app/package.json
- .github/workflows/meilisearch-prototype-tests.yml
- package.json
- .github/actions/set-up-node/action.yml
- .github/dependabot.yml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and the main CI workflow (.github/workflows/tests.yml) use `getmeili/meilisearch:latest` to maintain consistency between local development and CI testing environments.
Applied to files:
.github/workflows/tests.ymltests/env/browser/README.md.github/workflows/publish.ymltests/env/node/README.mdplaygrounds/javascript/README.md.github/workflows/pre-release-tests.ymltests/env/esm/package.json
📚 Learning: 2025-07-01T08:07:25.043Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 1977
File: docker-compose.yml:17-17
Timestamp: 2025-07-01T08:07:25.043Z
Learning: In the meilisearch-js project, both the docker-compose.yml and CI workflows use the latest Meilisearch image (getmeili/meilisearch:latest) for consistency between development and testing environments.
Applied to files:
.github/workflows/tests.ymltests/env/browser/README.mdtests/env/node/README.mdplaygrounds/javascript/README.md.github/workflows/pre-release-tests.yml
📚 Learning: 2025-09-04T06:44:01.679Z
Learnt from: flevi29
Repo: meilisearch/meilisearch-js PR: 2012
File: tests/client.test.ts:13-13
Timestamp: 2025-09-04T06:44:01.679Z
Learning: The meilisearch-js project supports Node.js 20 and above, so JSON imports should use the modern `with { type: "json" }` syntax rather than the deprecated `assert { type: "json" }` syntax.
Applied to files:
playgrounds/javascript/README.mdtests/env/esm/package.json
🪛 actionlint (1.7.9)
.github/workflows/tests.yml
39-39: could not parse as YAML: could not find expected ':'
(syntax-check)
.github/workflows/publish.yml
29-29: string should not be empty
(syntax-check)
29-29: unexpected end of input while parsing variable access, function call, null, bool, int, float or string. expecting "IDENT", "(", "INTEGER", "FLOAT", "STRING"
(expression)
🔇 Additional comments (14)
playgrounds/javascript/README.md (1)
10-10: Documentation accurately reflects pnpm migration.The README updates correctly document the shift from Yarn to pnpm for both installation and development workflows.
Also applies to: 16-16
tests/env/node/README.md (1)
9-9: Build command correctly updated to pnpm.Documentation now accurately reflects the use of pnpm for building the project.
tests/env/browser/README.md (1)
9-9: Build command correctly updated to pnpm.Documentation now accurately reflects the use of pnpm for building the project.
tests/env/esm/package.json (1)
8-8: Start script correctly uses pnpm for building.The script maintains the intended build-then-run pattern with pnpm.
.github/workflows/publish.yml (2)
22-27: Composite action and build step correctly configured.The use of
./.github/actions/set-up-nodewithregistry-urlfor npm publishing is appropriate, and thepnpm buildcommand is correct.
29-35: Publish steps correctly migrated to pnpm.Both prerelease and stable publish paths now use
pnpm publishwith consistent options (--provenance --access public). Unquoted conditionals are valid in GitHub Actions. Node auth token handling is preserved.Verify that the composite action at
./.github/actions/set-up-node/action.ymlis fully defined with required metadata (past review comments flagged a missingdescriptionfield). If not already fixed, ensure it includes:name: 'Set up Node.js and install dependencies' description: 'Configure Node.js and install project dependencies with pnpm' inputs: node-version: description: 'Node.js version' required: false default: 'lts/*' registry-url: description: 'Registry URL for npm' required: false default: '' runs: using: 'composite' steps: # ... steps ....github/workflows/tests.yml (4)
32-35: Environment variables correctly quoted and port mapping consistent.Updates to
MEILI_MASTER_KEY,MEILI_NO_ANALYTICS, and port mapping use double quotes consistently.
56-67: Test commands correctly migrated to pnpm.All test execution steps (
pnpm test,pnpm build,pnpm test:env:*) are correct. The implicit pnpm install (via composite action) eliminates redundant install steps.
76-80: Style check job properly uses composite action.Checkout (v6) followed by composite action setup and
pnpm stylecommand is correct.
88-95: Types check job properly uses composite action.Checkout (v6) followed by composite action setup and pnpm commands (
pnpm build,pnpm types) are correct..github/workflows/pre-release-tests.yml (4)
12-13: Branch patterns correctly quoted.Updated to double-quoted strings for consistency across workflow configuration.
Also applies to: 16-17
23-31: Meilisearch version extraction properly configured.Job outputs and grep-step with id are correctly set up to extract and export the prerelease version for downstream job consumption.
35-35: Job dependencies, environment, and matrix correctly migrated.
needssyntax updated to use double-quoted array- Environment variables use double quotes consistently
- Matrix key renamed to
node-versionwith versions["20", "22"]- Job name references
matrix.node-versionconsistentlyAlso applies to: 40-43, 46-47
50-65: Composite action and pnpm commands correctly configured.Checkout, composite action with
node-versioninput, and all pnpm test/build/env commands are properly set up. Version matrix values are consistent.
flevi29
force-pushed
the
yarn-to-pnpm-and-workflows
branch
from
cacf109 to
a872c48
Compare
flevi29
force-pushed
the
yarn-to-pnpm-and-workflows
branch
from
8b9d37f to
ecb99b1
Compare
3 participants
Pull Request
Related issue
Fixes #1660
What does this PR do
pnpmtests/env/*andplaygrounds/*packages now get their dependencies installed together with the main dependencies, because they are marked as part of the package (this does not affect publishing) (this also enables dependabot to track those dependencies as well)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.