-
Notifications
You must be signed in to change notification settings - Fork 92
Add script for manipulating and checking code samples #1948
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
Open
flevi29
wants to merge
12
commits into
meilisearch:main
Choose a base branch
from
flevi29:code-samples-safety
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ed03680
Add script for manipulating and checking code samples, refactor code …
flevi29 d801147
Improvements, add workflow step
flevi29 e4dccdd
Wrong workflows changed
flevi29 f8c398f
Fix trailing spaces problem
flevi29 f9a4286
Fix JSON formatting, add workflow format check, add comments
flevi29 ce84f37
Minor changes, update CONTRIBUTING.md
flevi29 0e76d6d
Update scripts/code-samples/to-yaml.js
flevi29 cf9419f
Minor changes
flevi29 3ed44ab
Merge
flevi29 f303e2e
Misc
flevi29 ab89914
Merge branch 'main' into code-samples-safety
flevi29 e147795
Refactor and improve
flevi29 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ | |
"build": "vite build && tsc -p tsconfig.build.json && vite --mode production-umd build", | ||
"postbuild": "node scripts/build.js", | ||
"test": "vitest run --coverage", | ||
"types": "tsc -p tsconfig.json --noEmit", | ||
"types": "yarn generate-code-sample-files && tsc -p tsconfig.json --noEmit", | ||
"types:watch": "yarn types --watch", | ||
"test:env:browser": "yarn build && node scripts/copy-umd-file.js --to ./tests/env/express/public && yarn --cwd tests/env/express && yarn --cwd tests/env/express test", | ||
"test:watch": "vitest watch", | ||
|
@@ -55,12 +55,14 @@ | |
"test:env:nodejs": "yarn build && node tests/env/node/index.cjs && node tests/env/node/getting_started.cjs", | ||
"test:env:esm": "yarn --cwd tests/env/esm && yarn --cwd tests/env/esm start", | ||
"test:env:nitro-app": "yarn build && yarn --cwd tests/env/nitro-app test", | ||
"generate-code-sample-files": "node scripts/code-samples.js from-yaml", | ||
"generate-code-samples-yaml": "node scripts/code-samples.js to-yaml", | ||
"fmt": "prettier -c ./**/*.{js,ts}", | ||
"fmt:fix": "prettier -w ./**/*.{js,ts}", | ||
"lint": "eslint", | ||
"lint:fix": "eslint --fix", | ||
"style": "yarn fmt && yarn lint", | ||
"style:fix": "yarn fmt:fix && yarn lint:fix", | ||
"style": "yarn generate-code-sample-files && prettier --ignore-path=.prettierignore -c generated-code-samples && yarn fmt && yarn lint", | ||
"style:fix": "yarn generate-code-sample-files && prettier --ignore-path=.prettierignore -c generated-code-samples && yarn fmt:fix && yarn lint:fix", | ||
"prepare": "husky" | ||
}, | ||
"files": [ | ||
|
@@ -70,22 +72,24 @@ | |
], | ||
"devDependencies": { | ||
"@eslint/js": "^9.23.0", | ||
"@vitest/coverage-v8": "^3.1.1", | ||
"@types/js-yaml": "^4.0.9", | ||
"@types/node": "^22.15.3", | ||
"@typescript-eslint/utils": "^8.29.0", | ||
"@vitest/coverage-v8": "^3.1.1", | ||
"@vitest/eslint-plugin": "^1.1.38", | ||
"eslint": "^9.25.1", | ||
"eslint-config-prettier": "^10.1.2", | ||
"eslint-plugin-tsdoc": "^0.4.0", | ||
"typescript": "^5.8.3", | ||
"vite": "^6.3.4", | ||
"globals": "^16.0.0", | ||
"husky": "^9.1.7", | ||
"js-yaml": "^4.1.0", | ||
"lint-staged": "15.5.1", | ||
"prettier": "^3.5.3", | ||
"prettier-plugin-jsdoc": "^1.3.2", | ||
"typedoc": "^0.28.3", | ||
"typescript": "^5.8.3", | ||
"typescript-eslint": "^8.31.1", | ||
"vite": "^6.3.4", | ||
"vitest": "^3.1.1" | ||
}, | ||
"packageManager": "[email protected]" | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { argv } from "node:process"; | ||
|
||
if (argv[2] === "to-yaml" && argv.length === 3) { | ||
await import("./code-samples/to-yaml.js"); | ||
} else if (argv[2] === "from-yaml") { | ||
await import("./code-samples/from-yaml.js"); | ||
} else { | ||
throw new Error( | ||
"expected `to-yaml` (+ new code samples names) or `from-yaml` as arguments", | ||
); | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
import { argv } from "node:process"; | ||
import { writeFileSync, mkdirSync } from "node:fs"; | ||
import { | ||
generatedCodeSamplesDir, | ||
iterateCodeSamples, | ||
delimiter, | ||
} from "./shared.js"; | ||
|
||
const headerImport = 'import { MeiliSearch } from "meilisearch";\n'; | ||
const headerClientDeclaration = | ||
'const client = new MeiliSearch({ host: "http://127.0.0.1:7700" });\n'; | ||
const headerComment = | ||
"// Code below this line will be written to code samples YAML file\n" + | ||
'// For more information consult CONTRIBUTING.md "Tests and Linter" section\n' + | ||
delimiter + | ||
"\n"; | ||
|
||
const jsonFilesToGenerate = ["games", "movies", "meteorites"]; | ||
|
||
try { | ||
mkdirSync(generatedCodeSamplesDir); | ||
} catch (error) { | ||
if (error.code !== "EEXIST") { | ||
throw error; | ||
} | ||
} | ||
|
||
// generate JSON files used by samples, so type check passes | ||
for (const jsonFileToGenerate of jsonFilesToGenerate) { | ||
writeFileSync( | ||
new URL(jsonFileToGenerate + ".json", generatedCodeSamplesDir), | ||
"[]\n", | ||
); | ||
} | ||
|
||
const clientVarRegExp = /(?<=const|let ).+(?= = new MeiliSearch\()/; | ||
|
||
let generatedFileTally = 0; | ||
|
||
for (const { sampleName, code } of iterateCodeSamples()) { | ||
let header = ""; | ||
|
||
const clientVarMatch = code.match(clientVarRegExp); | ||
const clientVarLiteral = clientVarMatch?.[0] ?? "client"; | ||
const clientVarUsageRegExp = new RegExp(`${clientVarLiteral}\\s*\\.`); | ||
|
||
// if there is client usage in the code sample | ||
if (clientVarUsageRegExp.test(code)) { | ||
// generate import if there isn't already one | ||
if (!code.includes('from "meilisearch";\n')) { | ||
header += headerImport; | ||
} | ||
|
||
// generate client declaration if there isn't already one | ||
if (clientVarMatch === null) { | ||
header += headerClientDeclaration; | ||
} | ||
} | ||
|
||
header += headerComment; | ||
|
||
writeFileSync( | ||
new URL(sampleName + ".ts", generatedCodeSamplesDir), | ||
header + code + "\n", | ||
); | ||
|
||
generatedFileTally += 1; | ||
} | ||
|
||
// generate additional files from arguments passed | ||
for (const sampleName of argv.slice(3)) { | ||
writeFileSync( | ||
new URL(sampleName + ".ts", generatedCodeSamplesDir), | ||
headerImport + headerClientDeclaration + headerComment, | ||
); | ||
|
||
generatedFileTally += 1; | ||
} | ||
flevi29 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
console.log(`generated ${generatedFileTally} code sample file(s)`); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import { readFileSync } from "node:fs"; | ||
import { load } from "js-yaml"; | ||
|
||
export const codeSamplesPath = new URL( | ||
"../../.code-samples.meilisearch.yaml", | ||
import.meta.url, | ||
); | ||
|
||
export const generatedCodeSamplesDir = new URL( | ||
"../../generated-code-samples/", | ||
import.meta.url, | ||
); | ||
|
||
export const delimiter = "// -~-~-~-~-"; | ||
|
||
export function* iterateCodeSamples() { | ||
const codeSamplesContents = readFileSync(codeSamplesPath); | ||
|
||
const codeSamples = load(codeSamplesContents, { | ||
filename: codeSamplesPath.href, | ||
onWarning: console.warn, | ||
}); | ||
flevi29 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (codeSamples === null || typeof codeSamples !== "object") { | ||
throw new Error("expected `codeSamples` to be an object", { | ||
cause: codeSamples, | ||
}); | ||
} | ||
|
||
for (const [sampleName, code] of Object.entries(codeSamples)) { | ||
if (typeof code !== "string") { | ||
throw new Error("expected `code` to be a string", { cause: code }); | ||
} | ||
|
||
yield { sampleName, code }; | ||
} | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
import { readdir } from "node:fs/promises"; | ||
import { readFileSync, writeFileSync } from "node:fs"; | ||
import { parse, join } from "node:path"; | ||
import { | ||
codeSamplesPath, | ||
generatedCodeSamplesDir, | ||
iterateCodeSamples, | ||
delimiter, | ||
} from "./shared.js"; | ||
|
||
const codeSampleNamesFromYaml = Array.from( | ||
iterateCodeSamples(), | ||
(v) => v.sampleName, | ||
); | ||
|
||
/** @type {import("node:fs").Dirent[]} */ | ||
const dirEntries = await readdir(generatedCodeSamplesDir, { | ||
withFileTypes: true, | ||
}).catch((error) => { | ||
if (error?.code !== "ENOENT") { | ||
throw error; | ||
} | ||
|
||
return []; | ||
}); | ||
|
||
if (dirEntries.length === 0) { | ||
throw new Error( | ||
`there are no code sample files at ${generatedCodeSamplesDir.href}\n` + | ||
"tip: first generate them from the YAML file, consult CONTRIBUTING.md on how to use this script", | ||
); | ||
} | ||
|
||
function throwError() { | ||
throw new Error( | ||
`expected generated code samples directory at ${generatedCodeSamplesDir.href} to only contain TypeScript and JSON files`, | ||
{ cause: dirEntries }, | ||
); | ||
} | ||
|
||
const manipulatedCodeSamples = dirEntries | ||
.map((dirEnt) => { | ||
if (!dirEnt.isFile()) { | ||
throwError(); | ||
} | ||
|
||
const { ext, name } = parse(dirEnt.name); | ||
if (ext !== ".ts" && ext !== ".json") { | ||
throwError(); | ||
} | ||
|
||
if (ext === ".json") { | ||
return null; | ||
} | ||
|
||
const codeSampleContent = readFileSync( | ||
join(dirEnt.parentPath, dirEnt.name), | ||
{ encoding: "utf-8" }, | ||
); | ||
|
||
const splitContent = codeSampleContent.split("\n"); | ||
const indexOfDelimiter = splitContent.findIndex((v) => v === delimiter); | ||
|
||
const indentedContent = splitContent | ||
// get rid of code before delimiter | ||
.slice(indexOfDelimiter === -1 ? 0 : indexOfDelimiter + 1) | ||
// add padding | ||
.map((v) => (v === "" ? v : " " + v)) | ||
.join("\n") | ||
.trimEnd(); | ||
|
||
// get position in current code samples YAML file, to be able to order it the same way | ||
const index = codeSampleNamesFromYaml.indexOf(name); | ||
|
||
return { name, indentedContent, index }; | ||
}) | ||
.filter((v) => v !== null) | ||
.sort(({ index: indexA }, { index: indexB }) => indexA - indexB); | ||
|
||
// for every new code sample, place them at the end of the file instead of the start | ||
while (manipulatedCodeSamples.at(0)?.index === -1) { | ||
manipulatedCodeSamples.push(manipulatedCodeSamples.shift()); | ||
} | ||
|
||
const serializedCodeSamples = manipulatedCodeSamples | ||
.map(({ name, indentedContent }) => name + ": |-\n" + indentedContent) | ||
.join("\n"); | ||
|
||
const header = | ||
"# This code-samples file is used by the Meilisearch documentation.\n" + | ||
"# Every example written here will be automatically fetched by\n" + | ||
"# the documentation on build.\n" + | ||
"# You can read more at https://github.com/meilisearch/documentation\n" + | ||
'# This file is generated, read more in CONTRIBUTING.md "Tests and Linter" section.\n' + | ||
"---\n"; | ||
|
||
writeFileSync(codeSamplesPath, header + serializedCodeSamples + "\n"); | ||
|
||
console.log(`generated ${manipulatedCodeSamples.length} code sample(s)`); |
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
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
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.
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.
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?
Uh oh!
There was an error while loading. Please reload this page.
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.
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
await
s 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.