-
Notifications
You must be signed in to change notification settings - Fork 67
Switch from yarn to pnpm pacakge manager #1428
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
base: main
Are you sure you want to change the base?
Conversation
|
WalkthroughAdds a pnpm-based workspace and migrates CI/docs from Yarn to pnpm, introduces a reusable GitHub Actions composite action to set up Node and install dependencies, updates workflows and package manifests to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 ignored due to path filters (1)
📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🪛 markdownlint-cli2 (0.18.1)playgrounds/local-react/README.md9-9: Fenced code blocks should have a language specified (MD040, fenced-code-language) 14-14: Fenced code blocks should have a language specified (MD040, fenced-code-language) playgrounds/react/README.md9-9: Fenced code blocks should have a language specified (MD040, fenced-code-language) 14-14: Fenced code blocks should have a language specified (MD040, fenced-code-language) playgrounds/javascript/README.md9-9: Fenced code blocks should have a language specified (MD040, fenced-code-language) 14-14: Fenced code blocks should have a language specified (MD040, fenced-code-language) playgrounds/autocomplete/README.md9-9: Fenced code blocks should have a language specified (MD040, fenced-code-language) 14-14: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🔇 Additional comments (4)
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 |
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/instant-meilisearch/README.md (1)
138-141: Avoid publishing realistic-looking API keys in README examples; use a placeholder + explicit warning. This reduces accidental leakage / copy-paste of privileged credentials.- 'a63da4928426f12639e19d62886f621130f3fa9ff3c7534c5d179f0f51c4f303' // API key + 'MEILISEARCH_SEARCH_API_KEY' // API key (use a search-only key; never expose admin/master keys)Also applies to: 168-171
playgrounds/autocomplete/README.md (1)
1-1: README title likely wrong for this playground (# React Playground). Consider renaming to match “Autocomplete Playground” to avoid confusion.packages/autocomplete-client/README.md (1)
91-95: Remove/replace the hard-coded API key in docs (treat as leaked secret).
Even if it’s “just an example”, it’s a real-looking key and will get scraped/copy-pasted. Use a placeholder and call out “search-only key”.- apiKey: 'a63da4928426f12639e19d62886f621130f3fa9ff3c7534c5d179f0f51c4f303' // API key + apiKey: '<YOUR_SEARCH_API_KEY>' // Search-only key (never use master/admin keys in the browser)playgrounds/local-react/package.json (1)
6-12: Document dev server dependency for local E2E testing.The E2E tests reference
baseUrl: http://localhost:5174(fromcypress.env.json), which requires the Vite dev server to be running. While CI handles this viacypress-io/github-action'sstart: pnpm playground:local-react, local developers runningpnpm test:e2ewill encounter flaky failures if the dev server is not already running. Add a note to CONTRIBUTING.md clarifying that developers must runpnpm devbefore executing E2E tests, or update the test scripts to usestart-server-and-testto automate this prerequisite.playgrounds/autocomplete/package.json (1)
6-12: E2E tests require dev server to be running but turbo.json does not establish this dependency.E2E tests expect the dev server at
http://localhost:5173(per cypress.env.json), yet thetest:e2etask in turbo.json only depends on"build"and not on"dev". Runningpnpm test:e2ewill fail because cypress cannot connect to a non-existent dev server. The GitHub workflows work around this by explicitly starting the playground via the cypress-io/github-action, but local or standard CI execution will not have the dev server running.Add
"dev"as a dependency to bothtest:e2eandtest:e2e:watchtasks in turbo.json to ensure the dev server starts before E2E tests run.
🧹 Nitpick comments (1)
playgrounds/local-react/README.md (1)
1-1: Optional: clarify the title to “Local React Playground” (currently generic).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
.github/actions/set-up-node/action.yml(1 hunks).github/dependabot.yml(1 hunks).github/workflows/dependabot-auto-merge.yml(0 hunks).github/workflows/meilisearch-prototype-tests.yml(3 hunks).github/workflows/pre-release-tests.yml(3 hunks).github/workflows/publish.yml(2 hunks).github/workflows/tests.yml(4 hunks)CONTRIBUTING.md(7 hunks)package.json(2 hunks)packages/autocomplete-client/README.md(1 hunks)packages/autocomplete-client/package.json(1 hunks)packages/instant-meilisearch/README.md(1 hunks)playgrounds/autocomplete/README.md(1 hunks)playgrounds/autocomplete/package.json(2 hunks)playgrounds/javascript/README.md(1 hunks)playgrounds/local-react/README.md(1 hunks)playgrounds/local-react/package.json(2 hunks)playgrounds/node-env/package.json(1 hunks)playgrounds/react/README.md(1 hunks)playgrounds/react/package.json(1 hunks)playgrounds/vue3/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/dependabot-auto-merge.yml
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[grammar] ~164-~164: Use a hyphen to join words.
Context: ... for all affected packages. See more in depth explaination on [versioning](https...
(QB_NEW_EN_HYPHEN)
[grammar] ~164-~164: Ensure spelling is correct
Context: ...l affected packages. See more in depth explaination on [versioning](https://github.com/chan...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
playgrounds/local-react/README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
CONTRIBUTING.md
99-99: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
playgrounds/autocomplete/README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/autocomplete-client/README.md
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
playgrounds/react/README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
playgrounds/javascript/README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (22)
playgrounds/vue3/package.json (1)
10-16: LGTM:workspace:*aligns with monorepo dependency resolution. Just ensure the workspace package exists / is included by the workspace globs.packages/instant-meilisearch/README.md (1)
116-126: Docs update topnpm addlooks good.playgrounds/react/package.json (1)
19-25: LGTM:workspace:*is the right move for a monorepo playground.playgrounds/local-react/package.json (1)
22-24:workspace:*in a private playground devDependency looks good..github/dependabot.yml (1)
10-41: Dependabot grouping looks good; double-check monthly cadence is intentional.
Switching npm updates to monthly can increase drift/security exposure; if intended, LGTM.playgrounds/autocomplete/package.json (1)
21-27:workspace:*dependency for the local playground is a good monorepo alignment.CONTRIBUTING.md (1)
29-53: Docs command updates to pnpm look consistent.packages/autocomplete-client/package.json (1)
43-45: The workspace protocol handling is already managed by your release tooling. The project uses@changesets/cli, which automatically convertsworkspace:*to actual semver versions during thechangeset publishcommand (as configured in the rootpackage.jsonrelease script). No additional changes are needed; this is the expected and proper configuration for monorepo publishing.package.json (2)
27-28: Scripts and package manager correctly migrated to pnpm.All yarn commands have been properly converted to pnpm equivalents. pnpm 10.25.0 is the latest version, and the packageManager field correctly specifies this for Corepack-based enforcement across all environments.
Also applies to: 30-31, 34-34, 36-36, 64-64
38-63: Verify that E2E tests function correctly after removingconcurrentlydependency.The removal of the
concurrentlypackage aligns with the simplified E2E test execution pattern described in the PR (no concurrent dev server startup). Ensure that the test execution flow in the updated workflows accommodates the sequential or alternative startup approach..github/workflows/publish.yml (2)
30-34: Changesets commands properly updated to pnpm.The
publishandversioncommands are correctly converted from yarn to pnpm, maintaining the same semantics.
19-19: The./.github/actions/set-up-nodecomposite action is properly implemented and correctly handles all Node.js setup, pnpm caching, and registry configuration. No changes needed..github/workflows/pre-release-tests.yml (3)
45-45: Verify that the./.github/actions/set-up-nodecomposite action correctly accepts and uses thenode-versioninput.The action is now passed a
node-versionparameter via thewith:block. Ensure the action properly configures the specified Node.js version before running tests.Also applies to: 80-80, 118-118, 120-120
114-114: Node.js version matrix expanded to 20, 22, 24.Node 18 has been removed from the test matrix. Confirm this aligns with your project's support policy—if Node 18 compatibility should still be maintained, it must remain in the matrix.
Also applies to: 115-115
52-52: pnpm commands correctly applied across test and playground execution.All yarn references have been properly substituted with pnpm equivalents for playground startup and test execution.
Also applies to: 87-87, 122-122
.github/workflows/meilisearch-prototype-tests.yml (3)
47-47: Composite action usage and node-version parameter handling are consistent with other workflows.These changes follow the same pattern as pre-release-tests.yml. Ensure the set-up-node action correctly handles the
node-versioninput parameter.Also applies to: 81-81, 118-118, 120-120
54-54: pnpm commands properly substituted across all test and playground invocations.Yarn to pnpm migration is consistent with the broader PR changes.
Also applies to: 88-88, 122-122
114-114: Node.js version matrix aligned with pre-release-tests.yml (versions 20, 22, 24).Confirm that dropping Node 18 aligns with your project's support policy.
Also applies to: 115-115
.github/workflows/tests.yml (4)
36-36: Composite action consistently used for Node.js and pnpm setup across all jobs.The set-up-node action is applied uniformly to all test jobs. Verify its implementation handles node-version input and pnpm cache correctly.
Also applies to: 80-80, 127-127, 141-141, 153-153
41-41: Clarify the wait-on configuration for cypress-autocomplete-client-run.Line 41 specifies
wait-on: 'http://localhost:7700,http://localhost:5173', whereas the cypress-instant-meilisearch-run job (line 85) only waits forhttp://localhost:7700. The inclusion of port 5173 (Vite dev server) appears inconsistent with the PR's goal of simplifying E2E test execution by removing concurrent dev server startup. Verify whether the Vite dev server on port 5173 is required for the autocomplete playground tests or if this can be removed.Also applies to: 43-43, 87-87
131-131: pnpm commands uniformly applied across all test jobs.All yarn references have been correctly converted to pnpm for test execution, build steps, and playground startup.
Also applies to: 135-135, 143-143, 155-155
123-123: Node.js version matrix expanded to 20, 22, 24; Node 18 removed.Confirm this aligns with your project's Node.js support policy. If Node 18 compatibility is still required, it must be added back to the matrix.
Also applies to: 124-124
| - name: Install pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| - name: Set up Node.js ${{ inputs.node-version }} | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: ${{ inputs.node-version }} | ||
| registry-url: ${{ inputs.registry-url }} | ||
| cache: "pnpm" | ||
| - name: Install dependencies | ||
| shell: bash | ||
| run: pnpm install |
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:
actions/setup-node current stable version 2024 2025
💡 Result:
As of Dec 13, 2025 the stable actions/setup-node release series is v6 (latest tag v6.1.0). Use it as: uses: actions/setup-node@v6. [1][2]
The action accepts version aliases like latest/current/node and explicit Node versions (e.g., 24, 24.8.0, lts/*). [1]
Sources:
[1] actions/setup-node README.
[2] actions/setup-node Releases (v6.1.0).
🌐 Web query:
pnpm/action-setup current stable version recommendations
💡 Result:
Short answer: use pnpm/action-setup@v4 (the current major maintained release). Prefer pinning the major tag (pnpm/action-setup@v4) or an exact patch/tag for reproducibility, and set the action input "version" (or rely on package.json's packageManager field) to the pnpm version you want. Example:
- Stable action: uses: pnpm/action-setup@v4. [1]
- For reproducible CI, pin a specific action tag or commit and specify with: with: version: 10 (or a specific pnpm semver like 10.22.1). [1][2]
Why: the repo/marketplace README explicitly shows and documents v4 as the current supported major and warns v2 is broken on newer Node.js — upgrade to v4 and avoid unpinned "latest" to prevent unexpected breakage. [1][2]
Sources:
[1] pnpm/action-setup README (GitHub).
[2] Setup pnpm — GitHub Marketplace.
🌐 Web query:
GitHub Actions pinning to commit SHA security best practice
💡 Result:
Short best-practice checklist for pinning GitHub Actions to commit SHAs
- Pin third‑party actions to the full-length commit SHA (not floating tags like @main or @v1). This is the most reliable way to make the action immutable. [1]
- Verify the SHA comes from the action’s original upstream repository (not a fork). Audit the action’s code (especially how it handles your repo/secrets) before pinning. [1][3]
- Enforce pinning centrally — use GitHub’s “allowed actions” / policy to require full SHA pins or block unpinned/unauthorized actions at repo/org/enterprise level. [2]
- Automate checks and updates:
- Use tooling (e.g., a workflow that fails if actions aren’t SHA‑pinned) and Dependabot or similar to propose vetted updates. Include a human review step before updating the SHA. [4][3]
- Add human-readable metadata: place the original tag/version and release URL in a comment next to the SHA (e.g., “# v1.2.3 — see release”), so reviewers know what the SHA corresponds to. [3][4]
- Where available, prefer immutable/attested releases (GitHub’s “immutable actions” / immutable releases) that allow safe tagging; otherwise default to SHA pinning. [2]
- Minimize exposure: prefer official/verified creators, maintain an allowlist of trusted action authors, and consider copying critical actions into your org (or using internal reusable workflows) if you must run them without constant re‑audits. [1][2][3]
If you want, I can:
- scan a repo/workflows and list unpinned actions, or
- show an example workflow that enforces SHA pinning.
🌐 Web query:
pnpm install --frozen-lockfile CI best practice
💡 Result:
Short best-practices for using pnpm --frozen-lockfile in CI
-
Commit a single canonical pnpm-lock.yaml for your repo (keep it up-to-date locally). pnpm will fail in CI if the lockfile is missing or out-of-sync. [1]
-
Pin/activate the pnpm version in CI (Corepack): corepack prepare pnpm@ --activate. Use the same pnpm version locally and in CI to avoid lockfile mismatches. [2][1]
-
In CI run: pnpm install --frozen-lockfile (or rely on CI default). This guarantees installs use the committed lockfile and prevents implicit lockfile changes during builds. [1]
-
If you need to update deps, do it outside CI (locally or in a dedicated job) and commit the updated pnpm-lock.yaml (or run pnpm install --lockfile-only in a controlled step/agent) — then open a PR. Avoid using --no-frozen-lockfile in regular CI. [1]
-
Cache the pnpm store or store-dir to speed CI (optional): configure pnpm_config_store_dir or pnpm config set store-dir and cache that directory per your CI docs. Caching is recommended but not required. [3]
-
For monorepos/workspaces: ensure all package.json changes and the workspace lockfile are kept in sync; CI will fail if any package manifest differs from the lockfile. Consider running automated pre-merge checks (pnpm install locally or pnpm install --lockfile-only) in PR pipelines. [1]
-
When debugging unexpected ERR_PNPM_OUTDATED_LOCKFILE errors, confirm pnpm versions and CI detection (env vars) and reproduce the install locally; if necessary run pnpm install to update the lockfile and commit. See known issues/PR guidance in pnpm repo. [4]
Sources
[1] pnpm install docs (frozen-lockfile behaviour and CI default).
[2] pnpm Continuous Integration guide (Corepack + pinning + caching examples).
[3] pnpm CI caching / store guidance.
[4] pnpm GitHub issue about CI/outdated-lockfile (debugging/mismatch context).
If you want, I can produce a minimal CI job (GitHub Actions / GitLab / Azure) that follows these practices.
Harden CI: pin actions to commit SHAs and use pnpm install --frozen-lockfile.
Actions v6 and pnpm/action-setup@v4 are current stable versions. For supply-chain security and reproducibility, pin both actions to their full-length commit SHAs (with version tags in comments) and ensure pnpm install uses --frozen-lockfile to enforce the committed lockfile in CI.
- uses: pnpm/action-setup@v4
+ uses: pnpm/action-setup@v4
+ # Consider pinning to commit SHA for reproducibility (e.g., v4.0.0 — <release-url>)
@@
- uses: actions/setup-node@v6
+ uses: actions/setup-node@v6
+ # Consider pinning to commit SHA for reproducibility (e.g., v6.1.0 — <release-url>)
@@
- run: pnpm install
+ run: pnpm install --frozen-lockfileCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/actions/set-up-node/action.yml around lines 13 to 23, the workflow
uses action references by tags which is less secure and currently runs `pnpm
install` without lockfile enforcement; pin both actions to their full commit
SHAs (keep the human-readable version tag in a comment for clarity) for
supply-chain security and replace the `pnpm install` run step with `pnpm install
--frozen-lockfile` so CI fails if lockfile and manifests are out of sync.
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.
pnpm uses version from main package.json, Node.js version is specified via input, otherwise it's latest LTS.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
|
||
| `Vanilla JS`: | ||
| ``` | ||
| yarn playground:javascript | ||
| pnpm playground:javascript | ||
| ``` | ||
|
|
||
| `HTML`: | ||
| ``` | ||
| yarn playground:html | ||
| pnpm playground:html | ||
| ``` |
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.
Add bash language to fenced blocks (MD040).
-```
+```bash
pnpm playground:javascript@@
- +bash
pnpm playground:html
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
99-99: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 97 to 106, the fenced code blocks for the
playground commands are missing a language specifier which triggers MD040;
update both blocks so they start with ```bash instead of plain ``` — i.e.,
change the first block containing pnpm playground:javascript to use ```bash and
change the second block containing pnpm playground:html to use ```bash so the
shells are properly highlighted.
| Each merge on `main` triggers the [`release` CI](./.github/workflows/publish.yml) generating a PR titled `Version Packages`. This PR updates the versions and contains changelogs of the impacted packages based on the `pnpm changesets` commands you ran on each PR. | ||
|
|
||
| To release on GitHub and `npm` you must merge this `Version packages` PR. This will trigger the publishing action and create the GitHub and `npm` releases for all affected packages. | ||
|
|
||
| See more in depth explaination on [versioning](https://github.com/changesets/changesets/blob/main/docs/command-line-options.md#version), [publishing](https://github.com/changesets/changesets/blob/main/docs/command-line-options.md#publish) and the [changesets github-action](https://github.com/changesets/action). | ||
|
|
||
| If you merged a beta branch, that was released, into main, you were probably in the `changesets` [pre-release](https://github.com/changesets/changesets/blob/main/docs/prereleases.md) mode (see section on [releasing a beta](#release-a-beta-version)). If the `pre.json` file is present in the `.changesets` folder, you need to exit that mode. This is possible by running `yarn changeset pre exit`. Once done, create a PR with the changes and merge it to main. | ||
| If you merged a beta branch, that was released, into main, you were probably in the `changesets` [pre-release](https://github.com/changesets/changesets/blob/main/docs/prereleases.md) mode (see section on [releasing a beta](#release-a-beta-version)). If the `pre.json` file is present in the `.changesets` folder, you need to exit that mode. This is possible by running `pnpm changeset pre exit`. Once done, create a PR with the changes and merge it to main. |
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.
Fix typos/wording: “more in-depth explanation”.
-See more in depth explaination on [versioning](
+See a more in-depth explanation on [versioning](🧰 Tools
🪛 LanguageTool
[grammar] ~164-~164: Use a hyphen to join words.
Context: ... for all affected packages. See more in depth explaination on [versioning](https...
(QB_NEW_EN_HYPHEN)
[grammar] ~164-~164: Ensure spelling is correct
Context: ...l affected packages. See more in depth explaination on [versioning](https://github.com/chan...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 160 to 166, the sentence "See more in depth
explaination on ..." contains typos and awkward wording; change it to a
grammatically correct phrase such as "See a more in-depth explanation on ..."
and correct "explaination" to "explanation", ensuring the surrounding links/text
remain unchanged.
| Use `npm` or `pnpm` to install the autocomplete client for Meilisearch. | ||
|
|
||
| ``` | ||
| yarn add @meilisearch/autocomplete-client | ||
| pnpm add @meilisearch/autocomplete-client | ||
| # or | ||
| npm install @meilisearch/autocomplete-client | ||
| ``` |
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.
Add a language to fenced code block to satisfy markdownlint (MD040).
-```
+```bash
pnpm add @meilisearch/autocomplete-client
# or
npm install @meilisearch/autocomplete-client
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In packages/autocomplete-client/README.md around lines 55 to 61, the fenced code
block is missing a language tag which triggers markdownlint MD040; update the
opening fence to include a language identifier (e.g., change tobash) so
the block becomes a bash code fence, and confirm both installation commands
remain inside that fenced block.
</details>
<!-- fingerprinting:phantom:triton:falcon -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ``` | ||
| yarn install | ||
| ``` | ||
|
|
||
| ### Compiles and hot-reloads for development | ||
| ``` | ||
| yarn dev | ||
| pnpm install | ||
| ``` | ||
|
|
||
| ### Compiles and minifies for production | ||
| ``` | ||
| yarn build | ||
| pnpm build | ||
| ``` |
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.
Fix markdownlint MD040: add a language to fenced code blocks.
-```
+```bash
pnpm install@@
- +bash
pnpm build
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
4-4: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In playgrounds/autocomplete/README.md around lines 4 to 11, the fenced code
blocks lack language identifiers which triggers markdownlint MD040; update each
triple-backtick fence to include the appropriate language tag (e.g., bash) so
the blocks read ```bash and ensure both code fences for the install and build
commands are updated consistently.
| ## Project setup | ||
| ``` | ||
| yarn install | ||
| ``` | ||
|
|
||
| ### Compiles and hot-reloads for development | ||
| ``` | ||
| yarn dev | ||
| pnpm install | ||
| ``` | ||
|
|
||
| ### Compiles and minifies for production | ||
| ``` | ||
| yarn build | ||
| pnpm build | ||
| ``` |
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.
Add languages to fenced blocks (MD040).
-```
+```bash
pnpm install@@
- +bash
pnpm build
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
4-4: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In playgrounds/javascript/README.md around lines 3 to 11, the fenced code blocks
are missing language identifiers which triggers MD040; update each
triple-backtick fence for the install and build commands to specify the language
(e.g., replace ``` with ```bash for both code blocks) so the fenced blocks
include the bash language tag.
| ``` | ||
| yarn install | ||
| ``` | ||
|
|
||
| ### Compiles and hot-reloads for development | ||
| ``` | ||
| yarn dev | ||
| pnpm install | ||
| ``` | ||
|
|
||
| ### Compiles and minifies for production | ||
| ``` | ||
| yarn build | ||
| pnpm build | ||
| ``` |
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.
Fix markdownlint MD040: add a language to fenced code blocks.
-```
+```bash
pnpm install@@
- +bash
pnpm build
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
4-4: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In playgrounds/local-react/README.md around lines 4 to 11, the fenced code
blocks lack language identifiers causing markdownlint MD040; add the language
(e.g., bash) to both fenced blocks so they read ```bash for the pnpm install
block and ```bash for the pnpm build block to satisfy the linter and enable
proper syntax highlighting.
| ``` | ||
| yarn install | ||
| ``` | ||
|
|
||
| ### Compiles and hot-reloads for development | ||
| ``` | ||
| yarn dev | ||
| pnpm install | ||
| ``` | ||
|
|
||
| ### Compiles and minifies for production | ||
| ``` | ||
| yarn build | ||
| pnpm build | ||
| ``` |
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.
Fix markdownlint MD040: add a language to fenced code blocks.
-```
+```bash
pnpm install@@
- +bash
pnpm build
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
4-4: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In playgrounds/react/README.md around lines 4 to 11 the fenced code blocks lack
a language specifier which triggers markdownlint MD040; update both code fences
to include a language (e.g., "bash") so they read ```bash instead of ``` to mark
the code as shell commands, leaving the inner content unchanged.
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 (2)
packages/autocomplete-client/__tests__/test.utils.ts (1)
2-2: Verify themeilisearch.MeiliSearchconstructor is part of the supported public API (and keep it DRY).
Ifmeilisearchis an implementation detail/re-export, this could break with package updates; also you can reuseHOST/API_KEYto avoid config drift between clients.-const meilisearchClient = new meilisearch.MeiliSearch({ - host: 'http://localhost:7700', - apiKey: 'masterKey', -}) +const meilisearchClient = new meilisearch.MeiliSearch({ + host: HOST, + apiKey: API_KEY, +})Also applies to: 15-18
playgrounds/autocomplete/package.json (1)
6-12: E2E scripts no longer start the dev server—verify CI/docs cover the new contract.
If the intent is “run Cypress against an already-running Vite server,” this is OK, but it’s a behavioral change that can surprise local users unless workflows/docs clearly startpnpm dev(or similar) beforehand.
📜 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)
packages/autocomplete-client/__tests__/test.utils.ts(2 hunks)playgrounds/autocomplete/package.json(2 hunks)playgrounds/autocomplete/setup.mjs(1 hunks)playgrounds/local-react/setup.mjs(1 hunks)playgrounds/node-env/index.js(1 hunks)playgrounds/vue3/src/App.vue(1 hunks)vite.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- playgrounds/vue3/src/App.vue
🧰 Additional context used
🧬 Code graph analysis (4)
playgrounds/autocomplete/setup.mjs (1)
playgrounds/local-react/setup.mjs (1)
client(5-8)
packages/autocomplete-client/__tests__/test.utils.ts (1)
packages/instant-meilisearch/__tests__/assets/utils.ts (1)
meilisearchClient(330-330)
playgrounds/local-react/setup.mjs (1)
playgrounds/autocomplete/setup.mjs (1)
client(4-7)
playgrounds/node-env/index.js (2)
packages/instant-meilisearch/src/client/instant-meilisearch-client.ts (1)
instantMeiliSearch(46-212)playgrounds/react/src/App.jsx (1)
instantMeiliSearch(18-24)
🔇 Additional comments (4)
vite.config.ts (1)
4-7: The currenttest.projects: ['packages/*']configuration is correct and follows Vitest best practices.Vitest 3.0.2 (your repo's version) accepts glob patterns like
packages/*to discover project directories. This glob is standard and properly matches the two packages in your repository. Both packages havevite.config.tsfiles, which Vitest auto-discovers without requiring separatevitest.config.tsfiles. The suggested change topackages/*/vitest.config.{ts,js,mjs,cjs}would not match any files in your setup and is unnecessary. The migration from the deprecatedtest.workspacetotest.projectsis also correctly applied.Likely an incorrect or invalid review comment.
playgrounds/node-env/index.js (1)
1-4: Constructor switch looks consistent; verify Node runs this file as ESM (top-levelawait).
Themeilisearch.MeiliSearchchange matches the new import shape; just ensure this playground is executed in an environment where top-levelawaitis valid forindex.js(e.g.,type: "module"or equivalent).Also applies to: 11-14
playgrounds/local-react/setup.mjs (1)
1-1: LGTM; please sanity-check the@meilisearch/instant-meilisearchexport shape in this repo’s pinned version.
This is a clean mechanical swap, but it depends onmeilisearchbeing a stable named export andmeilisearch.MeiliSearchbeing available.Also applies to: 5-8
playgrounds/autocomplete/package.json (1)
21-30:workspace:*dependency wiring makes sense for a pnpm monorepo; verify publish boundaries.
Since this package is private it’s likely fine, but ensure nothing intended for publishing relies onworkspace:*versions (and confirm@meilisearch/instant-meilisearchis always available in the workspace graph).
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)
playgrounds/javascript/package.json (1)
14-16: Consider clarifying workspace dependency context in playground documentation.The
workspace:*protocol correctly resolves within the pnpm workspace (confirmed by config:pnpm-workspace.yamlincludesplaygrounds/*andpackages/*). The dependency will fail clearly outside the workspace with an appropriate error, as designed. However, the CONTRIBUTING.md and playgrounds/javascript/README.md could explicitly mention thatpnpm installmust be run from the repository root (not the playground directory alone) for@meilisearch/instant-meilisearchto resolve. Adding a note like "Install from the workspace root:pnpm install" would prevent contributor confusion around the workspace:* dependency requirement.
📜 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 (9)
.github/actions/set-up-node/action.yml(1 hunks)eslint.config.js(1 hunks)package.json(1 hunks)packages/instant-meilisearch/__tests__/filter.test.ts(1 hunks)playgrounds/autocomplete/package.json(2 hunks)playgrounds/javascript/package.json(1 hunks)playgrounds/local-react/package.json(2 hunks)playgrounds/react/package.json(1 hunks)playgrounds/vue3/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/instant-meilisearch/tests/filter.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- playgrounds/vue3/package.json
- playgrounds/autocomplete/package.json
- playgrounds/react/package.json
- .github/actions/set-up-node/action.yml
🔇 Additional comments (5)
eslint.config.js (1)
90-100: Address the TODO to remove disabled vitest rules; no violations currently exist in the test suite.The
vitest/no-conditional-expectdisable aligns with the existingTODO: Remove rulescomment. Since no violations of this rule (or the other two disabled rules) were found across 27 test files, removing all three disables is safe and recommended:// Vitest { files: ['packages/*/{src,__tests__}/**/*.test.ts'], extends: [vitest.configs.recommended], - // TODO: Remove rules - rules: { - 'vitest/no-identical-title': 'off', - 'vitest/valid-title': 'off', - 'vitest/no-conditional-expect': 'off', - }, },If removing all three causes test failures, enable the failing rule(s) selectively with inline
// eslint-disable-next-lineand a brief reason.playgrounds/local-react/package.json (2)
6-12: Cypress scripts now assume workspace/root toolchain—verifyturbo/pnpm filtering still works.
If CI/dev runs these viaturbo test:e2efrom the repo root, this is fine; but running the package in isolation may now fail unless the repo-level install is enforced. Consider documenting “run from repo root” (or adding a guard in scripts).
22-24: Dropping React/Vite deps here is OK only if intentionally centralized—please confirm no standalone install path.
Right now this playground depends on root workspace deps to build/run; if that’s the intent, LGTM—just ensure docs/CI reflect it and no tooling expects per-package installs.package.json (2)
27-36: Scripts look consistent; verify they run correctly under pnpm in CI (esp. build chaining).
style/style:fix/test/test:watch/test:types/releasenow assume pnpm is the invoker; please ensure CI usespnpm -w(orcorepack pnpm) and thatpnpm build(root script) behaves as expected across platforms.
73-73: CI already honorspnpm@10.25.0viapnpm/action-setup.The workflows use
pnpm/action-setup@v4, which automatically respects thepackageManagerfield inpackage.jsonwithout requiring explicit corepack or version configuration. The pinned version is enforced in CI.For local development, consider documenting in
CONTRIBUTING.md(or similar) that contributors should either enable corepack or explicitly installpnpm@10.25.0to match the pinned version, avoiding accidental use of a different major version.
| "@vitest/eslint-plugin": "^1.1.23", | ||
| "algoliasearch": "^5.19.0", | ||
| "algoliasearch-helper": "^3.23.0", | ||
| "cypress": "^8.6.0", |
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:
Cypress 8.6.0 Node.js compatibility requirements
💡 Result:
Cypress 8.6.0 requires Node.js >= 12.0.0 (package.json "engines": "node": ">=12.0.0"). [1][2]
Sources:
- cypress v8.6.0 package.json (engines). [1]
- Cypress changelog / migration docs describing Node.js 12+ support for that era. [2]
Cypress 8.6.0 is outdated (from 2021) but has broad Node.js support (>= 12.0.0).
Since Cypress 8 supports Node 12+, a Node upgrade in CI should not cause breakage. If this version is intentionally pinned for legacy reasons, consider documenting that. If upgrading is an option, consider moving to a more recent version.
Pull Request
What does this PR do?
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.