Add script for manipulating and checking code samples #1948
Conversation
flevi29
added
documentation
WalkthroughThis update introduces a new workflow for managing, generating, and type-checking Meilisearch JavaScript/TypeScript code samples. It adds scripts to convert between YAML and TypeScript code samples, integrates these steps into build and lint scripts, updates documentation, and modernizes all code samples to use consistent async/await patterns. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Script as code-samples.js
participant FromYaml as from-yaml.js
participant ToYaml as to-yaml.js
participant TS as TypeScript Compiler
Dev->>Script: yarn generate-code-sample-files
Script->>FromYaml: Import and execute
FromYaml->>TS: Generate .ts files for samples
Dev->>Script: yarn generate-code-samples-yaml
Script->>ToYaml: Import and execute
ToYaml->>FromYaml: Read .ts files
ToYaml->>Script: Write .yaml file
Dev->>TS: yarn types (runs generate-code-sample-files first)
TS->>FromYaml: Type checks generated .ts files
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1948 +/- ##
=======================================
Coverage 99.03% 99.03%
=======================================
Files 18 18
Lines 1449 1449
Branches 305 305
=======================================
Hits 1435 1435
Misses 14 14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
CONTRIBUTING.md (1)
73-76: Fix grammatical issues in the documentationThe added documentation is informative, but contains a few grammatical issues.
- In this repository code samples are linted and type checked. To achieve this we generate - TypeScript files from `.code-samples.meilisearch.yaml`, and vice-versa. + In this repository, code samples are linted and type checked. To achieve this, we generate + TypeScript files from `.code-samples.meilisearch.yaml`, and vice versa.🧰 Tools
🪛 LanguageTool
[uncategorized] ~75-~75: Possible missing comma found.
Context: ...rn build ``` ### Code samples In this repository code samples are linted and type checke...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~75-~75: Possible missing comma found.
Context: ...are linted and type checked. To achieve this we generate TypeScript files from `.cod...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~76-~76: The expression “vice versa” is spelled without hyphens.
Context: ...m.code-samples.meilisearch.yaml, and vice-versa. ```bash # Generate files yarn generat...(VICE_VERSA)
scripts/code-samples/from-yaml.js (2)
18-18: Consider externalizing the list of JSON filesThe hard-coded list of JSON files could be moved to the shared module to ensure consistency between scripts.
-const jsonFilesToGenerate = ["games", "movies", "meteorites"]; +import { jsonFilesToGenerate } from "./shared.js";And in shared.js:
export const jsonFilesToGenerate = ["games", "movies", "meteorites"];
29-34: Consider more robust JSON file generationThe script currently generates empty arrays for JSON files, but it might be better to generate more realistic mock data or at least structured objects with the expected properties.
for (const jsonFileToGenerate of jsonFilesToGenerate) { + const mockData = { + games: [{ id: "1", name: "Example Game", genre: "RPG" }], + movies: [{ id: "1", title: "Example Movie", director: "Jane Doe" }], + meteorites: [{ id: "1", name: "Example Meteorite", mass: 1000 }] + }; + + const content = jsonFileToGenerate in mockData + ? JSON.stringify(mockData[jsonFileToGenerate], null, 2) + : "[]"; + writeFileSync( new URL(jsonFileToGenerate + ".json", generatedCodeSamplesDir), - "[]\n", + content + "\n", ); }.code-samples.meilisearch.yaml (5)
1-4: Add immediate documentation for code-samples workflow
The PR notes that CONTRIBUTING.md updates are planned but not yet included. Adding a placeholder or brief instructions now will help contributors use the new scripts without confusion.
15-16: Ensure TypeScript JSON import support in tsconfig
The samples useimport games from "./games.json" with { type: "json" }. Confirm that yourtsconfig.jsonhasresolveJsonModule,esModuleInterop, and a compatiblemoduleResolutionso the generated.tsfiles compile successfully.
328-329: Consider addingawaitfor thehealth()call
Inget_health_1,_client.health()returns a promise; prefixing it withawaitaligns with other examples and ensures errors aren't silently dropped.- const _health = _client.health(); + const _health = await _client.health();
459-463: Inconsistent_taskvariable naming
Theadd_movies_json_1sample usesconst taskinstead of the underscore-prefixedconst _taskconvention used throughout. For consistency, rename it:- const task = await _client.index("movies").addDocuments(movies).waitTask(); + const _task = await _client.index("movies").addDocuments(movies).waitTask();
5-1019: Refactor and modularize large code-samples YAML
With over 100 entries in a single file, maintenance and navigation become difficult. Consider splitting by domain (indexes, documents, search, settings, security) into separate YAML files or using YAML anchors/shared literals to reduce duplication and improve clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
.code-samples.meilisearch.yaml(3 hunks).gitignore(1 hunks)CONTRIBUTING.md(1 hunks)package.json(3 hunks)scripts/code-samples.js(1 hunks)scripts/code-samples/from-yaml.js(1 hunks)scripts/code-samples/shared.js(1 hunks)scripts/code-samples/to-yaml.js(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/code-samples/from-yaml.js (2)
scripts/code-samples/shared.js (5)
delimiter(14-14)delimiter(14-14)generatedCodeSamplesDir(9-12)generatedCodeSamplesDir(9-12)iterateCodeSamples(16-37)scripts/code-samples/to-yaml.js (1)
header(88-94)
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~75-~75: Possible missing comma found.
Context: ...rn build ``` ### Code samples In this repository code samples are linted and type checke...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~75-~75: Possible missing comma found.
Context: ...are linted and type checked. To achieve this we generate TypeScript files from `.cod...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~76-~76: The expression “vice versa” is spelled without hyphens.
Context: ...m .code-samples.meilisearch.yaml, and vice-versa. ```bash # Generate files yarn generat...
(VICE_VERSA)
🔇 Additional comments (15)
.gitignore (1)
136-136: Adding generated code samples to gitignore looks goodThis addition prevents generated code sample files from being committed to the repository, which is the correct approach for generated artifacts.
tsconfig.json (1)
11-11: Good addition of verbatimModuleSyntaxEnabling
verbatimModuleSyntax: trueenforces stricter type checking for imports/exports, which is beneficial for catching errors at compile time.CONTRIBUTING.md (1)
77-85: Commands section looks goodThe commands section clearly explains how to generate code sample files and YAML files, which is helpful for contributors.
scripts/code-samples.js (1)
1-11: Script logic looks goodThe script serves as a clear entry point for the code sample generation workflow. The logic correctly routes to the appropriate implementation based on command-line arguments and provides informative error messages.
Some positive aspects:
- Clean use of dynamic imports
- Proper error handling with descriptive messages
- Top-level await is used correctly
package.json (4)
47-47: Integration of code sample generation with type checkingThe
typesscript now depends ongenerate-code-sample-files, which ensures that TypeScript code samples are generated before type checking occurs. This is a good workflow improvement that will catch type errors in code samples early.
58-59: Well-defined scripts for bidirectional code sample conversionThese new scripts create a clear workflow for maintaining code samples, allowing conversion from YAML to TypeScript files and vice versa. This facilitates both type checking and documentation generation.
64-65: Style checking now includes generated code samplesThe style scripts now check the
generated-code-samplesdirectory, ensuring consistent formatting across all code, including samples. The pattern of generating files before checking them is consistent with the approach in thetypesscript.
75-75: Appropriate dependencies for YAML processingAddition of
js-yamland its type definitions is necessary for the new code sample processing workflow. The versions chosen are appropriate and stable.Also applies to: 85-85
scripts/code-samples/to-yaml.js (3)
11-14: Good approach for preserving sample orderReading the existing sample names from the YAML file first allows maintaining the original ordering, which is important for documentation consistency.
27-32: Helpful error message for empty directory caseThe error message clearly explains the problem and provides a tip for resolution, which is excellent for developer experience.
79-82: Clever approach for sample orderingThis code effectively moves new samples (with index -1) to the end of the file while preserving the original order of existing samples. Using array methods like
at()andshift()makes the code concise.scripts/code-samples/shared.js (2)
4-12: Good use of URL for portable path resolutionUsing the
URLconstructor withimport.meta.urlis an excellent approach for resolving paths relative to the module location, ensuring the script works correctly regardless of the working directory.
16-37: Well-implemented generator for code sample iterationThe generator function provides a clean API for iterating through code samples with proper error handling. The error messages include the problematic values as
cause, which is excellent for debugging.scripts/code-samples/from-yaml.js (2)
20-26: Good error handling for directory creationThe script only ignores the expected "directory already exists" error (
EEXIST) and properly rethrows any other errors, which is a robust approach.
42-49: Smart detection for existing imports and client declarationsThe script intelligently adds import statements and client declarations only when needed, which avoids duplicating code and preserves any custom implementations in the samples.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Hi, I think that the code samples are a bit less readable now.
Why the underscore before the name of every variable?
Furthermore, the examples used to be simple method calls, but now they also contain variable assignments.
I don't think this is needed, and it doesn't align with the Meilisearch documentation practices, see example.
Can we revert these changes?
There was a problem hiding this comment.
Yeah, alright. I reverted it.
I can see the problem, in particular that no other library does this, and they all sort of use the same pattern, where possible, but here is my thought process anyhow for why I did what I did.
My intention was to write linter and formatter compliant code samples in addition to being type checked. One of the solutions to this was to add variables with underscores (_client), because these variables don't always get used, which makes the linter err.
As for variable assignments, I thought it would be more clear that the method call returns something that we want to use, because there are method calls which do not return anything (Promise<void>), and the variable names indicate clearly what is being returned.
I also added .waitTask() initially, to indicate that the method call results in an enqueued task, that has to be awaited in almost every situation at some point in the code.
I went ahead and reverted these changes.
I did leave the awaits though, removing those would require for us to ignore a rule from the linter, and I believe it clearly signals that we're dealing with a promise.
Pull Request
Related issue
Fixes #1947
What does this PR do?
Summary by CodeRabbit
New Features
Documentation
Chores
Style