Rename to lower case s Meilisearch#1474
Draft
flevi29 wants to merge 3 commits into
Draft
Conversation
🦋 Changeset detectedLatest commit: 771f2ce The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughRepository-wide identifier and type renames to standardize casing: Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/instant-meilisearch/src/client/instant-meilisearch-client.ts (1)
88-103:⚠️ Potential issue | 🟠 MajorComplete the casing migration for public return/options fields.
meiliSearchInstanceandmeiliSearchParamsstill use old uppercase-Scasing while the setter was already renamed tosetMeilisearchParams. Since this PR is a breaking casing migration, leaving these public fields behind maintains mixed old/new API names.Update across all layers:
- Type definitions (
meiliSearchInstance,meiliSearchParamsin types.ts)- Implementation (instant-meilisearch-client.ts lines 88-103)
- Tests (search-resolver.test.ts, overridden-meilisearch-parameters.test.ts, search-params.test.ts, hit-adapter.test.ts)
- Documentation (README.md examples)
- Adapters (search-params-adapter.ts, search-response-adapter.ts, hits-adapter.ts)
- CHANGELOG references
Suggested direction
return { - meiliSearchInstance: meilisearchClient, + meilisearchInstance: meilisearchClient, setMeilisearchParams: (params): void => { - const { meiliSearchParams } = instantMeilisearchOptions + const { meilisearchParams } = instantMeilisearchOptions - instantMeilisearchOptions.meiliSearchParams = - meiliSearchParams === undefined + instantMeilisearchOptions.meilisearchParams = + meilisearchParams === undefined ? params : { - ...meiliSearchParams, + ...meilisearchParams, ...params, indexesOverrides: { - ...(meiliSearchParams.indexesOverrides || {}), + ...(meilisearchParams.indexesOverrides || {}), ...(params.indexesOverrides || {}), }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instant-meilisearch/src/client/instant-meilisearch-client.ts` around lines 88 - 103, Public API fields meiliSearchInstance and meiliSearchParams still use the old uppercase "S" while the setter was renamed to setMeilisearchParams; rename the public fields to meilisearchInstance and meilisearchParams (lowercase "s") everywhere to complete the casing migration, updating their type definitions (types.ts), the implementation in instant-meilisearch-client.ts (the object property currently named meiliSearchInstance and instantMeilisearchOptions.meiliSearchParams), all tests (search-resolver.test.ts, overridden-meilisearch-parameters.test.ts, search-params.test.ts, hit-adapter.test.ts), adapter files (search-params-adapter.ts, search-response-adapter.ts, hits-adapter.ts), README examples, and any CHANGELOG references so all imports/uses match the new names and the setter setMeilisearchParams continues to operate on the renamed meilisearchParams field.packages/instant-meilisearch/__tests__/disjunctive-facet-search.test.ts (1)
7-7:⚠️ Potential issue | 🟡 MinorStray TODO comment should be removed before merge.
// TODO: re-read for reviewlooks like a leftover author note rather than meaningful context for future maintainers.Proposed fix
-// TODO: re-read for review - describe('Keep zero facets tests', () => {As per coding guidelines: "Prefer self-descriptive code to comments. Only use comments for complex logic."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instant-meilisearch/__tests__/disjunctive-facet-search.test.ts` at line 7, Remove the stray author note comment "// TODO: re-read for review" from the test file so the test code is self-descriptive; open packages/instant-meilisearch/__tests__/disjunctive-facet-search.test.ts, locate the exact comment and delete it (no replacement needed) to satisfy the coding guideline about avoiding leftover TODOs.packages/instant-meilisearch/README.md (1)
401-460:⚠️ Potential issue | 🟡 MinorUpdate
meiliSearchParamstomeilisearchParamsfor consistency or document the legacy casing.The function names follow lowercase convention (
instantMeilisearch,setMeilisearchParams), but the option key remainsmeiliSearchParamswith a capital 'S'. This inconsistency spans the entire codebase—options, types, tests, and docs. Either rename it tomeilisearchParamsthroughout for uniform casing, or if preserving the old spelling is intentional for backward compatibility, add a migration note in the docs clarifying why this particular key was exempted from the rename.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instant-meilisearch/README.md` around lines 401 - 460, The README and codebase use the inconsistent option key meiliSearchParams (capital S) while function names use lowercase (instantMeilisearch, setMeilisearchParams); either rename the option key to meilisearchParams everywhere (types, tests, docs, function signatures and any parsing/validation of options in instantMeilisearch and related helpers) to ensure uniform camelCase, or explicitly document the legacy casing by adding a migration note in the docs and keeping a compatibility layer in instantMeilisearch that accepts both meiliSearchParams and meilisearchParams (map the legacy key to the new one before further processing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/instant-meilisearch/__tests__/assets/utils.ts`:
- Line 2: The import uses the wrong exported symbol name: update the import in
utils.ts to match the installed meilisearch version by either upgrading the
meilisearch dependency to v0.57.0+ so the lowercase-exported Meilisearch is
available, or change the import to use the existing exported symbol MeiliSearch
(capital S); locate the import line that currently references Meilisearch and
replace it accordingly so the symbol name matches the package version.
In `@packages/instant-meilisearch/src/client/config/index.ts`:
- Around line 11-17: Update the stale JSDoc for getInstantMeilisearchConfig to
use the correct parameter name: change the `@param` tag from "option" to "options"
and ensure the type {InstantMeilisearchOptions} and return
{InstantMeilisearchConfig} remain accurate so the doc matches the function
signature getInstantMeilisearchConfig(options: InstantMeilisearchOptions).
In `@packages/instant-meilisearch/src/types/types.ts`:
- Around line 75-82: The public option keys meiliSearchParams and
meiliSearchInstance were left with the old camel casing and should be renamed to
meilisearchParams and meilisearchInstance to match the rest of the API; update
the BaseInstantMeilisearchOptions type (replace meiliSearchParams with
meilisearchParams) and the other affected type/interface around the
meiliSearchInstance declaration (replace meiliSearchInstance with
meilisearchInstance), and search the repo for any remaining references to the
old meiliSearch* symbols to rename them or add aliases so the exported types and
runtime options consistently use meilisearch*.
---
Outside diff comments:
In `@packages/instant-meilisearch/__tests__/disjunctive-facet-search.test.ts`:
- Line 7: Remove the stray author note comment "// TODO: re-read for review"
from the test file so the test code is self-descriptive; open
packages/instant-meilisearch/__tests__/disjunctive-facet-search.test.ts, locate
the exact comment and delete it (no replacement needed) to satisfy the coding
guideline about avoiding leftover TODOs.
In `@packages/instant-meilisearch/README.md`:
- Around line 401-460: The README and codebase use the inconsistent option key
meiliSearchParams (capital S) while function names use lowercase
(instantMeilisearch, setMeilisearchParams); either rename the option key to
meilisearchParams everywhere (types, tests, docs, function signatures and any
parsing/validation of options in instantMeilisearch and related helpers) to
ensure uniform camelCase, or explicitly document the legacy casing by adding a
migration note in the docs and keeping a compatibility layer in
instantMeilisearch that accepts both meiliSearchParams and meilisearchParams
(map the legacy key to the new one before further processing).
In `@packages/instant-meilisearch/src/client/instant-meilisearch-client.ts`:
- Around line 88-103: Public API fields meiliSearchInstance and
meiliSearchParams still use the old uppercase "S" while the setter was renamed
to setMeilisearchParams; rename the public fields to meilisearchInstance and
meilisearchParams (lowercase "s") everywhere to complete the casing migration,
updating their type definitions (types.ts), the implementation in
instant-meilisearch-client.ts (the object property currently named
meiliSearchInstance and instantMeilisearchOptions.meiliSearchParams), all tests
(search-resolver.test.ts, overridden-meilisearch-parameters.test.ts,
search-params.test.ts, hit-adapter.test.ts), adapter files
(search-params-adapter.ts, search-response-adapter.ts, hits-adapter.ts), README
examples, and any CHANGELOG references so all imports/uses match the new names
and the setter setMeilisearchParams continues to operate on the renamed
meilisearchParams field.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02ae1fd1-0209-44e0-9f6a-60c124da79f7
📒 Files selected for processing (37)
.changeset/wild-cooks-stand.mdpackage.jsonpackages/autocomplete-client/__tests__/test.utils.tspackages/autocomplete-client/src/client/createSearchClient.tspackages/autocomplete-client/src/types/ClientConfig.tspackages/autocomplete-client/src/types/MeilisearchOptions.tspackages/autocomplete-client/src/types/SearchClient.tspackages/instant-meilisearch/README.mdpackages/instant-meilisearch/__tests__/assets/utils.tspackages/instant-meilisearch/__tests__/custom-http-client.test.tspackages/instant-meilisearch/__tests__/disjunctive-facet-search.test.tspackages/instant-meilisearch/__tests__/first-facets-distribution.test.tspackages/instant-meilisearch/__tests__/instantiation.test.tspackages/instant-meilisearch/__tests__/multi-index-search.test.tspackages/instant-meilisearch/__tests__/overridden-meilisearch-parameters.test.tspackages/instant-meilisearch/__tests__/placeholder-search.test.tspackages/instant-meilisearch/__tests__/search-metadata.test.tspackages/instant-meilisearch/__tests__/search-resolver.test.tspackages/instant-meilisearch/src/adapter/search-request-adapter/__tests__/search-params.test.tspackages/instant-meilisearch/src/adapter/search-request-adapter/search-params-adapter.tspackages/instant-meilisearch/src/adapter/search-request-adapter/search-resolver.tspackages/instant-meilisearch/src/adapter/search-response-adapter/hits-adapter.tspackages/instant-meilisearch/src/adapter/search-response-adapter/search-response-adapter.tspackages/instant-meilisearch/src/cache/init-facets-distribution.tspackages/instant-meilisearch/src/client/config/index.tspackages/instant-meilisearch/src/client/instant-meilisearch-client.tspackages/instant-meilisearch/src/contexts/search-context.tspackages/instant-meilisearch/src/types/types.tsplaygrounds/autocomplete/setup.mjsplaygrounds/javascript/src/app.jsplaygrounds/local-react/setup.mjsplaygrounds/local-react/src/components/MultiIndex.jsxplaygrounds/local-react/src/components/SingleIndex.jsxplaygrounds/local-react/src/components/SingleMovieIndex.jsxplaygrounds/node-env/index.jsplaygrounds/react/src/App.jsxplaygrounds/vue3/src/App.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
What does this PR do?
Migration
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
Refactor
Documentation
Chores