feat: implement emoji generation #4
Conversation
luxass
changed the title
WalkthroughThis commit enhances the emoji processing system. The adapter functions now include robust error handling and expanded metadata support while shifting to a unified context parameter. The CLI command has been refactored—renaming and consolidating options for multiple generators—and additional utilities, version management functions, caching improvements, and testing suites have been added. Interfaces and types have been updated accordingly, with new modules for GitHub shortcode generation and Unicode processing. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as CLI ("generate")
participant ADAP as Moji Adapter
participant NET as Cache/Network
participant FS as File System
U->>CLI: Run "generate <versions...>" with options
CLI->>CLI: Validate options (--generators, --shortcode-providers)
CLI->>ADAP: Invoke generator functions (metadata, sequences, emojis, variations, shortcodes)
ADAP->>NET: Fetch data via cache (using ctx.emojiVersion)
NET-->>ADAP: Return emoji data or errors
ADAP-->>CLI: Return processed emoji output (or throw MojisNotImplemented)
CLI->>FS: Write output JSON files to designated directories
sequenceDiagram
participant SC as GitHub Shortcode Generator
participant NET as Cache/Network
participant API as GitHub API
SC->>NET: Request cached GitHub emoji data (using version & force parameters)
NET->>API: Fetch data if cache is missing or force enabled
API-->>NET: Return GitHub emoji JSON
NET-->>SC: Return cached/fetched data
SC->>SC: Process data (extract hexcode via regex, validate against provided emojis)
SC-->>Caller: Return list of EmojiShortcode
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/adapter/base.ts(2 hunks)src/cli.ts(2 hunks)src/utils/errors.ts(1 hunks)
🔇 Additional comments (6)
src/utils/errors.ts (1)
1-10: LGTM! Well-structured error handling implementation.The error handling implementation follows TypeScript best practices with:
- A properly extended Error class
- A correctly implemented type guard using type predicates
src/adapter/base.ts (3)
2-2: LGTM! Clean imports for error handling.The imports are well-organized, separating color utilities from error types.
Also applies to: 6-6
8-12: LGTM! Improved error message formatting.The error message now includes color formatting for better visibility.
68-69: LGTM! Consistent adapter method implementation.The new methods follow the same pattern as the existing
sequencesmethod.src/cli.ts (2)
67-77: LGTM! Robust error handling implementation.The error handling distinguishes between not-implemented warnings and other errors, providing appropriate feedback for each case.
Also applies to: 127-137
59-64: LGTM! Safe file operations.The implementation correctly ensures directory existence before writing files and uses proper JSON formatting.
Also applies to: 119-124
src/cli.ts
Outdated
| cli.command( | ||
| "generate:emojis <versions...>", | ||
| "Generate emoji data for the specified versions", | ||
| (args) => commonOptions(args) | ||
| .positional("versions", { | ||
| type: "string", | ||
| description: "emoji versions to generate", | ||
| }) | ||
| .strict().help(), | ||
| async (args) => { | ||
| const force = args.force ?? false; | ||
| const versions = Array.isArray(args.versions) ? args.versions : [args.versions]; | ||
|
|
||
| if (SUPPORTED_EMOJI_VERSIONS.every((v) => !versions.includes(v))) { | ||
| console.error(red("error:"), "unsupported emoji versions"); | ||
| console.log("supported versions:", SUPPORTED_EMOJI_VERSIONS.join(", ")); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| console.log("generating emoji data for versions", versions.map((v) => yellow(v)).join(", ")); | ||
|
|
||
| const promises = versions.map(async (version) => { | ||
| const coerced = semver.coerce(version); | ||
|
|
||
| if (coerced == null) { | ||
| throw new Error(`invalid version ${version}`); | ||
| } | ||
|
|
||
| const adapter = resolveAdapter(coerced.version); | ||
|
|
||
| if (adapter == null) { | ||
| throw new Error(`no adapter found for version ${version}`); | ||
| } | ||
|
|
||
| const emojis = await adapter.emojis!({ version, force }); | ||
|
|
||
| await fs.ensureDir(`./data/v${version}`); | ||
| return fs.writeFile( | ||
| `./data/v${version}/emojis.json`, | ||
| JSON.stringify(emojis, null, 2), | ||
| "utf-8", | ||
| ); | ||
| }); | ||
|
|
||
| const results = await Promise.allSettled(promises); | ||
|
|
||
| for (const result of results) { | ||
| if (result.status === "rejected") { | ||
| if (isNotImplementedError(result.reason)) { | ||
| console.warn(yellow("warning:"), result.reason.message); | ||
| continue; | ||
| } | ||
| console.error(red("error:"), result.reason); | ||
| } | ||
| } | ||
|
|
||
| console.log(green("done")); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting common command logic to reduce duplication.
The generate:emojis and generate:variations commands share significant code. Consider extracting the common logic into a reusable function to improve maintainability.
Here's a suggested refactor:
+async function generateData(
+ versions: string[],
+ force: boolean,
+ type: 'emojis' | 'variations',
+ adapterMethod: 'emojis' | 'variations'
+) {
+ if (SUPPORTED_EMOJI_VERSIONS.every((v) => !versions.includes(v))) {
+ console.error(red("error:"), "unsupported emoji versions");
+ console.log("supported versions:", SUPPORTED_EMOJI_VERSIONS.join(", "));
+ process.exit(1);
+ }
+
+ console.log(`generating emoji ${type} for versions`, versions.map((v) => yellow(v)).join(", "));
+
+ const promises = versions.map(async (version) => {
+ const coerced = semver.coerce(version);
+
+ if (coerced == null) {
+ throw new Error(`invalid version ${version}`);
+ }
+
+ const adapter = resolveAdapter(coerced.version);
+
+ if (adapter == null) {
+ throw new Error(`no adapter found for version ${version}`);
+ }
+
+ const data = await adapter[adapterMethod]!({ version, force });
+
+ await fs.ensureDir(`./data/v${version}`);
+ return fs.writeFile(
+ `./data/v${version}/${type}.json`,
+ JSON.stringify(data, null, 2),
+ "utf-8",
+ );
+ });
+
+ const results = await Promise.allSettled(promises);
+
+ for (const result of results) {
+ if (result.status === "rejected") {
+ if (isNotImplementedError(result.reason)) {
+ console.warn(yellow("warning:"), result.reason.message);
+ continue;
+ }
+ console.error(red("error:"), result.reason);
+ }
+ }
+
+ console.log(green("done"));
+}
cli.command(
"generate:emojis <versions...>",
"Generate emoji data for the specified versions",
(args) => commonOptions(args)
.positional("versions", {
type: "string",
description: "emoji versions to generate",
})
.strict().help(),
async (args) => {
- // ... existing implementation ...
+ const force = args.force ?? false;
+ const versions = Array.isArray(args.versions) ? args.versions : [args.versions];
+ await generateData(versions, force, 'emojis', 'emojis');
},
);
cli.command(
"generate:variations <versions...>",
"Generate emoji variations for the specified versions",
(args) => commonOptions(args)
.positional("versions", {
type: "string",
description: "emoji versions to generate",
})
.strict().help(),
async (args) => {
- // ... existing implementation ...
+ const force = args.force ?? false;
+ const versions = Array.isArray(args.versions) ? args.versions : [args.versions];
+ await generateData(versions, force, 'variations', 'variations');
},
);Also applies to: 83-141
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/cli.ts (1)
23-141: 🛠️ Refactor suggestionConsider extracting common command logic to reduce duplication.
The
generate:emojisandgenerate:variationscommands share significant code. The suggestion to extract common logic into a reusable function is still valid.
🧹 Nitpick comments (5)
src/adapter/v16.ts (1)
72-135: Consider improving error handling & clarifying TODO
- If
fetchCachefails due to network or parse errors, consider adding fallback or retry logic to prevent unhandled rejections.- You have a
TODO: use correct unicode version(line 113). Let me know if you would like assistance in implementing a more dynamic approach to determine the version.- If
extractEmojiVersionreturnsnull, the call toextractUnicodeVersion(emojiVersion, "16.0")may produce unexpected results. Ensure that scenario is valid or handled explicitly.src/adapter/base.ts (2)
104-104: Address the TODO comment about unicode version.The hardcoded version "16.0" in
extractUnicodeVersionshould be replaced with the correct version from the context.Would you like me to help implement the correct unicode version extraction?
71-108: Consider enhancing error handling for malformed data.The parsing of emoji-test.txt could be more robust. Consider:
- Adding try-catch blocks around the parsing logic.
- Validating the format of extracted data before using it.
Apply this diff to enhance error handling:
const [baseHexcode, trailingLine] = line.split(";"); -if (baseHexcode == null || trailingLine == null) { +if (!baseHexcode?.trim() || !trailingLine?.trim()) { throw new Error(`invalid line: ${line}`); } const [baseQualifier, comment] = trailingLine.split("#"); -if (baseQualifier == null || comment == null) { +if (!baseQualifier?.trim() || !comment?.trim()) { throw new Error(`invalid line: ${line}`); } +try { const hexcode = baseHexcode.trim().replace(/\s+/g, "-"); const qualifier = baseQualifier.trim(); const emojiVersion = extractEmojiVersion(comment.trim()); const [emoji, trimmedComment] = comment.trim().split(` E${emojiVersion} `); const groupName = currentGroup?.slug ?? "unknown"; const subgroupName = currentGroup?.subgroups[currentGroup.subgroups.length - 1] ?? "unknown"; const metadataGroup = `${groupName}-${subgroupName}`; if (emojiMetadata[metadataGroup] == null) { emojiMetadata[metadataGroup] = {}; } emojiMetadata[metadataGroup][hexcode] = { group: groupName, subgroup: subgroupName, qualifier, emojiVersion: emojiVersion || null, unicodeVersion: extractUnicodeVersion(emojiVersion, "16.0"), description: trimmedComment || "", emoji: emoji || null, hexcodes: hexcode.split("-"), }; +} catch (error) { + console.error(`Error processing line: ${line}`); + throw error; +}src/utils.ts (2)
283-291: Consider enhancing the version extraction regex.The current regex
/E(\d+\.\d)/might miss some edge cases. Consider:
- Supporting optional patch versions (e.g., E14.0.1)
- Ensuring the 'E' is not part of another word
Apply this diff to enhance the regex pattern:
-const version = comment.match(/E(\d+\.\d)/); +const version = comment.match(/(?:^|[^a-zA-Z])E(\d+\.\d+(?:\.\d+)?)/);
294-332: Consider using a version map for better maintainability.The switch statement for version mapping could be replaced with a map for better maintainability and readability.
Apply this diff to use a version map:
+const EMOJI_TO_UNICODE_VERSION_MAP = new Map([ + ['0.7', '7.0'], + ['1.0', '8.0'], + ['2.0', '8.0'], + ['3.0', '9.0'], + ['4.0', '9.0'], + ['5.0', '10.0'], +]); export function extractUnicodeVersion(emojiVersion: string | null, unicodeVersion?: string): string | null { const coercedEmojiVersion = semver.coerce(emojiVersion); const coercedUnicodeVersion = semver.coerce(unicodeVersion); if (coercedEmojiVersion == null || coercedUnicodeVersion == null) { return null; } // v11+ aligned emoji and unicode specs (except for minor versions) if (semver.gte(coercedEmojiVersion, "11.0.0")) { // if the unicode version is not provided, we will return the emoji version. if (unicodeVersion == null) { return emojiVersion; } // return the smallest version between the emoji and unicode version. if (semver.lt(coercedEmojiVersion, coercedUnicodeVersion)) { return emojiVersion; } return unicodeVersion; } - switch (emojiVersion) { - case "0.7": - return "7.0"; - case "1.0": - case "2.0": - return "8.0"; - case "3.0": - case "4.0": - return "9.0"; - case "5.0": - return "10.0"; - default: - // v6 is the first unicode spec emojis appeared in - return "6.0"; - } + return EMOJI_TO_UNICODE_VERSION_MAP.get(emojiVersion ?? '') ?? '6.0'; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/adapter/base.ts(2 hunks)src/adapter/index.ts(2 hunks)src/adapter/v16.ts(3 hunks)src/cli.ts(6 hunks)src/types.ts(1 hunks)src/utils.ts(1 hunks)test/utils.test.ts(2 hunks)
🔇 Additional comments (10)
src/adapter/v16.ts (3)
1-1: Imports look good
No issues identified.
4-4: New utility imports recognized
Nice job importingextractEmojiVersionandextractUnicodeVersionfor your emoji processing logic.
32-32: Comment alignment
Your in-code comment about skipping empty lines and comments aligns with the code below.test/utils.test.ts (2)
2-2: Additional imports
Recognized the new import forextractEmojiVersion. No issues noted.
34-52: Coverage for valid & invalid versions
Your tests cover multiple scenarios, including valid inputs, invalid formats, and whitespace handling. This thorough approach is appreciated.src/adapter/index.ts (2)
40-42: New optional properties
Introducingshortcodesandmetadatais a clean extension of the adapter interface.
53-59: Verify removal of legacy references
You’ve addedShortcodeFnandMetadataFnwhile removinggroups. Please confirm that no remaining code references the oldgroupsproperty.src/types.ts (2)
20-29: LGTM! Well-structured interface for emoji metadata.The interface is well-designed with appropriate types for all properties, including nullable version fields and an array for hexcodes to support composite emojis.
31-38: LGTM! Well-structured interface for emoji data.The interface is well-designed with appropriate types for all properties, including nullable version fields and an array of Property type for emoji properties.
src/adapter/base.ts (1)
8-12: LGTM! Enhanced error handling with colored output.The function now uses the specialized
MojisNotImplementederror type and enhances error messages with red color for better visibility.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/adapter/index.ts (2)
40-42: Add JSDoc documentation for new adapter methods.The new
shortcodesandmetadatamethods lack documentation explaining their purpose and return types.Add JSDoc comments similar to other methods:
+ /** + * A function to generate emoji shortcodes for the specified version. + */ shortcodes?: ShortcodeFn; + /** + * A function to generate emoji metadata and groups for the specified version. + */ metadata?: MetadataFn;
46-48: Document version properties in BaseAdapterContext.The
emojiVersionandunicodeVersionproperties lack documentation explaining their purpose and format.Add JSDoc comments:
export interface BaseAdapterContext { + /** + * The emoji version (e.g., "15.0") + */ emojiVersion: string; + /** + * The unicode version (e.g., "15.0.0") + */ unicodeVersion: string; force: boolean; }src/adapter/base.ts (2)
19-20: Improve version check readability.The version check uses magic numbers. Consider using constants to improve maintainability.
+const VERSIONS_WITHOUT_GROUP_DATA = ["1.0", "2.0", "3.0"]; + - if (ctx.emojiVersion === "1.0" || ctx.emojiVersion === "2.0" || ctx.emojiVersion === "3.0") { + if (VERSIONS_WITHOUT_GROUP_DATA.includes(ctx.emojiVersion)) {
71-81: Extract parsing logic into helper functions.The line parsing logic is complex and could benefit from being broken down into smaller, reusable functions.
+function parseLine(line: string): { hexcode: string, qualifier: string, comment: string } { + const [baseHexcode, trailingLine] = line.split(";"); + if (baseHexcode == null || trailingLine == null) { + throw new Error(`invalid line: ${line}`); + } + + const [baseQualifier, comment] = trailingLine.split("#"); + if (baseQualifier == null || comment == null) { + throw new Error(`invalid line: ${line}`); + } + + return { + hexcode: baseHexcode.trim().replace(/\s+/g, "-"), + qualifier: baseQualifier.trim(), + comment: comment.trim() + }; +} - const [baseHexcode, trailingLine] = line.split(";"); - if (baseHexcode == null || trailingLine == null) { - throw new Error(`invalid line: ${line}`); - } - const [baseQualifier, comment] = trailingLine.split("#"); - if (baseQualifier == null || comment == null) { - throw new Error(`invalid line: ${line}`); - } - const hexcode = baseHexcode.trim().replace(/\s+/g, "-"); - const qualifier = baseQualifier.trim(); + const { hexcode, qualifier, comment } = parseLine(line);src/adapter/v16.ts (1)
72-135: Extract common parsing logic and improve type safety.The emoji data parsing logic contains repeated patterns and could benefit from stronger typing.
+interface ParsedLine { + hex: string; + property: string; + lineComment: string; +} + +function parseLine(line: string): ParsedLine | null { + if (line.trim() === "" || line.startsWith("#")) { + return null; + } + + const lineCommentIndex = line.indexOf("#"); + const lineComment = lineCommentIndex !== -1 + ? line.slice(lineCommentIndex + 1).trim() + : ""; + + let [hex, property] = line.split(";").map((col) => col.trim()).slice(0, 4); + + if (hex == null || property == null) { + throw new Error(`invalid line: ${line}`); + } + + const propertyCommentIndex = property.indexOf("#"); + if (propertyCommentIndex !== -1) { + property = property.slice(0, propertyCommentIndex).trim(); + } + + return { hex, property, lineComment }; +} async emojis(ctx) { const emojiData = await fetchCache(`https://unicode.org/Public/${ctx.emojiVersion}.0/ucd/emoji/emoji-data.txt`, { cacheKey: `v${ctx.emojiVersion}/emoji-data.json`, parser(data) { const lines = data.split("\n"); const emojiData: Record<string, EmojiData> = {}; for (const line of lines) { - // skip empty line & comments - if (line.trim() === "" || line.startsWith("#")) { - continue; - } - - const lineCommentIndex = line.indexOf("#"); - const lineComment = lineCommentIndex !== -1 ? line.slice(lineCommentIndex + 1).trim() : ""; - - let [hex, property] = line.split(";").map((col) => col.trim()).slice(0, 4); - - if (hex == null || property == null) { - throw new Error(`invalid line: ${line}`); - } - - // remove line comment from property - const propertyCommentIndex = property.indexOf("#"); - if (propertyCommentIndex !== -1) { - property = property.slice(0, propertyCommentIndex).trim(); - } + const parsed = parseLine(line); + if (parsed == null) continue; + const { hex, property, lineComment } = parsed; if (property === "Extended_Pictographic") { continue; }src/utils.ts (1)
293-332: Improve function documentation.The function implementation looks good, but the documentation needs improvement:
- The unicode.org reference should be in JSDoc format
- Missing parameter type documentation
- Missing return value documentation
Apply this diff to enhance the documentation:
-// https://unicode.org/reports/tr51/#EmojiVersions +/** + * Maps emoji versions to their corresponding unicode versions. + * Based on the specification at {@link https://unicode.org/reports/tr51/#EmojiVersions}. + * + * @param {string | null} emojiVersion - The emoji version to map + * @param {string} [unicodeVersion] - Optional unicode version for comparison + * @returns {string | null} The corresponding unicode version, or null if versions are invalid + * + * @example + * ```ts + * extractUnicodeVersion("11.0") // returns "11.0" + * extractUnicodeVersion("5.0") // returns "10.0" + * extractUnicodeVersion(null) // returns null + * ``` + */ export function extractUnicodeVersion(emojiVersion: string | null, unicodeVersion?: string): string | null {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/adapter/base.ts(2 hunks)src/adapter/index.ts(2 hunks)src/adapter/v16.ts(4 hunks)src/cli.ts(7 hunks)src/utils.ts(1 hunks)
🔇 Additional comments (1)
src/cli.ts (1)
23-141: Previous review comment is still valid: Extract common command logic.The
generate:emojisandgenerate:variationscommands share significant code. The previous suggestion to extract common logic remains applicable.
src/cli.ts
Outdated
| const { groups, emojiMetadata } = await adapter.metadata!({ emojiVersion: version, force, unicodeVersion: getUnicodeVersionByEmojiVersion(version)! }); | ||
|
|
||
| await fs.ensureDir(`./data/v${version}`); | ||
| return fs.writeFile( | ||
| await fs.ensureDir(`./data/v${version}/metadata`); | ||
|
|
||
| await fs.writeFile( | ||
| `./data/v${version}/groups.json`, | ||
| JSON.stringify(groups, null, 2), | ||
| "utf-8", | ||
| ); | ||
|
|
||
| return Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( | ||
| `./data/v${version}/metadata/${group}.json`, | ||
| JSON.stringify(metadata, null, 2), | ||
| "utf-8", | ||
| )); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for file operations.
The metadata file operations lack proper error handling for file system operations.
const { groups, emojiMetadata } = await adapter.metadata!({ emojiVersion: version, force, unicodeVersion: getUnicodeVersionByEmojiVersion(version)! });
- await fs.ensureDir(`./data/v${version}/metadata`);
+ try {
+ await fs.ensureDir(`./data/v${version}/metadata`);
+
+ await fs.writeFile(
+ `./data/v${version}/groups.json`,
+ JSON.stringify(groups, null, 2),
+ "utf-8",
+ );
- await fs.writeFile(
- `./data/v${version}/groups.json`,
- JSON.stringify(groups, null, 2),
- "utf-8",
- );
- return Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile(
- `./data/v${version}/metadata/${group}.json`,
- JSON.stringify(metadata, null, 2),
- "utf-8",
- ));
+ return Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile(
+ `./data/v${version}/metadata/${group}.json`,
+ JSON.stringify(metadata, null, 2),
+ "utf-8",
+ ));
+ } catch (error) {
+ throw new Error(`Failed to write metadata files: ${error.message}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { groups, emojiMetadata } = await adapter.metadata!({ emojiVersion: version, force, unicodeVersion: getUnicodeVersionByEmojiVersion(version)! }); | |
| await fs.ensureDir(`./data/v${version}`); | |
| return fs.writeFile( | |
| await fs.ensureDir(`./data/v${version}/metadata`); | |
| await fs.writeFile( | |
| `./data/v${version}/groups.json`, | |
| JSON.stringify(groups, null, 2), | |
| "utf-8", | |
| ); | |
| return Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( | |
| `./data/v${version}/metadata/${group}.json`, | |
| JSON.stringify(metadata, null, 2), | |
| "utf-8", | |
| )); | |
| }); | |
| const { groups, emojiMetadata } = await adapter.metadata!({ emojiVersion: version, force, unicodeVersion: getUnicodeVersionByEmojiVersion(version)! }); | |
| try { | |
| await fs.ensureDir(`./data/v${version}/metadata`); | |
| await fs.writeFile( | |
| `./data/v${version}/groups.json`, | |
| JSON.stringify(groups, null, 2), | |
| "utf-8", | |
| ); | |
| return Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( | |
| `./data/v${version}/metadata/${group}.json`, | |
| JSON.stringify(metadata, null, 2), | |
| "utf-8", | |
| )); | |
| } catch (error) { | |
| throw new Error(`Failed to write metadata files: ${error.message}`); | |
| } | |
| }); |
src/utils.ts
Outdated
| export function getUnicodeVersionByEmojiVersion(emojiVersion: string): string { | ||
| const coercedEmojiVersion = semver.coerce(emojiVersion); | ||
|
|
||
| if (coercedEmojiVersion == null) { | ||
| throw new Error(`invalid emoji version: ${emojiVersion}`); | ||
| } | ||
|
|
||
| if (semver.gte(coercedEmojiVersion, "11.0.0")) { | ||
| return emojiVersion; | ||
| } | ||
|
|
||
| switch (emojiVersion) { | ||
| case "1.0": | ||
| case "2.0": | ||
| return "8.0"; | ||
| case "3.0": | ||
| case "4.0": | ||
| return "9.0"; | ||
| case "5.0": | ||
| return "10.0"; | ||
| default: | ||
| throw new Error(`invalid emoji version: ${emojiVersion}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation and handle missing case.
The function needs the following improvements:
- Missing JSDoc documentation
- Error messages could be more descriptive
- Missing case "0.7" that exists in
extractUnicodeVersion
Apply this diff to improve the implementation:
+/**
+ * Gets the unicode version for a given emoji version.
+ * Throws an error if the emoji version is invalid.
+ *
+ * @param {string} emojiVersion - The emoji version to get unicode version for
+ * @returns {string} The corresponding unicode version
+ * @throws {Error} If the emoji version is invalid or unsupported
+ *
+ * @example
+ * ```ts
+ * getUnicodeVersionByEmojiVersion("11.0") // returns "11.0"
+ * getUnicodeVersionByEmojiVersion("5.0") // returns "10.0"
+ * getUnicodeVersionByEmojiVersion("invalid") // throws Error
+ * ```
+ */
export function getUnicodeVersionByEmojiVersion(emojiVersion: string): string {
const coercedEmojiVersion = semver.coerce(emojiVersion);
if (coercedEmojiVersion == null) {
- throw new Error(`invalid emoji version: ${emojiVersion}`);
+ throw new Error(`Invalid emoji version format: ${emojiVersion}. Expected format: major.minor[.patch]`);
}
if (semver.gte(coercedEmojiVersion, "11.0.0")) {
return emojiVersion;
}
switch (emojiVersion) {
+ case "0.7":
+ return "7.0";
case "1.0":
case "2.0":
return "8.0";
case "3.0":
case "4.0":
return "9.0";
case "5.0":
return "10.0";
default:
- throw new Error(`invalid emoji version: ${emojiVersion}`);
+ throw new Error(`Unsupported emoji version: ${emojiVersion}. Only versions 0.7 through 5.0 and 11.0+ are supported.`);
}
}
src/utils.ts
Outdated
| /** | ||
| * Extracts the emoji version from a comment string. | ||
| * The version should be in the format "E{major}.{minor}" (e.g. "E14.0"). | ||
| * | ||
| * @param {string} comment - The comment string to extract the version from | ||
| * @returns {string | null} The parsed version number, or null if no valid version was found | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * extractEmojiVersion("E14.0") // returns "14.0" | ||
| * extractEmojiVersion("Something else") // returns null | ||
| * ``` | ||
| */ | ||
| export function extractEmojiVersion(comment: string): string | null { | ||
| const version = comment.match(/E(\d+\.\d)/); | ||
|
|
||
| if (version != null && version[1] != null) { | ||
| return version[1].trim(); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance version extraction robustness.
The current implementation has the following limitations:
- The regex pattern is too restrictive:
- Does not handle patch versions (e.g., "E14.0.1")
- Does not handle single-digit versions (e.g., "E1.0")
- Missing input validation for null/undefined
Apply this diff to improve the implementation:
export function extractEmojiVersion(comment: string): string | null {
+ if (comment == null) {
+ return null;
+ }
- const version = comment.match(/E(\d+\.\d)/);
+ const version = comment.match(/E(\d+\.\d+(?:\.\d+)?)/);
if (version != null && version[1] != null) {
return version[1].trim();
}
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Extracts the emoji version from a comment string. | |
| * The version should be in the format "E{major}.{minor}" (e.g. "E14.0"). | |
| * | |
| * @param {string} comment - The comment string to extract the version from | |
| * @returns {string | null} The parsed version number, or null if no valid version was found | |
| * | |
| * @example | |
| * ```ts | |
| * extractEmojiVersion("E14.0") // returns "14.0" | |
| * extractEmojiVersion("Something else") // returns null | |
| * ``` | |
| */ | |
| export function extractEmojiVersion(comment: string): string | null { | |
| const version = comment.match(/E(\d+\.\d)/); | |
| if (version != null && version[1] != null) { | |
| return version[1].trim(); | |
| } | |
| return null; | |
| } | |
| /** | |
| * Extracts the emoji version from a comment string. | |
| * The version should be in the format "E{major}.{minor}" (e.g. "E14.0"). | |
| * | |
| * @param {string} comment - The comment string to extract the version from | |
| * @returns {string | null} The parsed version number, or null if no valid version was found | |
| * | |
| * @example | |
| * |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/utils/cache.ts (1)
63-64: Consider using a proper logging system.While the cache hit logging is helpful, using
console.logdirectly might not be the best approach for production code. Consider implementing a proper logging system that can be configured based on the environment.- // eslint-disable-next-line no-console - console.log(`cache hit: ${green(cacheKey)}`); + logger.debug(`cache hit: ${green(cacheKey)}`);src/adapter/base.ts (3)
70-74: Enhance error message with line details.The error message could be more helpful by including the specific part that's missing.
- throw new Error(`invalid line: ${line}`); + throw new Error(`invalid line (missing hexcode or trailing content): ${line}`);
91-91: Extract group-subgroup concatenation to a utility function.Consider extracting the group-subgroup concatenation logic to a reusable utility function.
+function getMetadataGroupKey(group: string, subgroup: string): string { + return `${group}-${subgroup}`; +} + - const metadataGroup = `${groupName}-${subgroupName}`; + const metadataGroup = getMetadataGroupKey(groupName, subgroupName);
133-133: Add input validation for hex values.Consider validating that hex values are in the correct format.
+ if (!/^[0-9A-F]+$/i.test(hex)) { + throw new Error(`invalid hex value: ${hex}`); + } + unicodeNames[hex] = name;src/adapter/v16.ts (2)
122-122: Consider using a Set for properties from the start.Initialize properties as a Set to avoid the need for deduplication later.
- properties: [(property as Property) || "Emoji"], + properties: new Set([property as Property || "Emoji"]),
171-173: Extract style types as constants.Consider extracting the style types as constants to improve maintainability.
+const VALID_STYLES = ["text", "emoji"] as const; + - if (type !== "text" && type !== "emoji") { + if (!VALID_STYLES.includes(type as typeof VALID_STYLES[number])) { throw new Error(`invalid style: ${style}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/adapter/base.ts(2 hunks)src/adapter/index.ts(3 hunks)src/adapter/v16.ts(4 hunks)src/cli.ts(7 hunks)src/types.ts(1 hunks)src/utils/cache.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types.ts
🔇 Additional comments (10)
src/utils/cache.ts (2)
3-3: LGTM!The import of the
greenfunction enhances the visibility of cache hit messages in the console.
60-60: LGTM!The addition of the type parameter
<TData>toreadCacheimproves type safety by ensuring the cached data matches the expected type.src/adapter/index.ts (4)
1-1: LGTM!The import statement correctly includes all necessary types for emoji data handling.
40-44: LGTM!The new optional methods enhance the adapter's capabilities for emoji generation, providing flexibility for different implementations.
48-49: LGTM!The separation of emoji and Unicode versions makes the version handling more explicit and accurate.
97-102: LGTM!The
MojisNotImplementederror class provides clear and specific error handling for unimplemented adapter methods.src/adapter/base.ts (1)
9-9: LGTM!The use of
MojisNotImplementedwith colored output improves error visibility and specificity.src/cli.ts (3)
23-88: Extract common command logic to reduce duplication.The
generate:emojiscommand shares significant code withgenerate:variations. Consider extracting the common logic into a reusable function.
67-71: Verify the empty object write.An empty object is being written to
emojis.json. Is this intentional?
251-263: Add error handling for file operations.The metadata file operations lack proper error handling for file system operations.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/types.ts (1)
18-21: 💡 Verification agent🧩 Analysis chain
Consider implementing the empty interface.
The
EmojiComponentinterface is currently empty. Consider defining its properties or removing it if unused.
🏁 Script executed:
#!/bin/bash # Search for usages of EmojiComponent rg "EmojiComponent" -A 5Length of output: 429
Action Required: Define or Document the
EmojiComponentInterface
- The interface is referenced in the array
components: EmojiComponent[];within the emoji-related type definitions, indicating it’s intended to play a role.- Since the interface is currently empty, please either populate it with the required properties or, if it’s intentionally left as a placeholder, add a clarifying comment explaining its intended future use to avoid confusion.
🧰 Tools
🪛 Biome (1.9.4)
[error] 19-21: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🧹 Nitpick comments (2)
src/adapter/base.ts (1)
70-106: Consider breaking down the metadata parsing logic.The metadata parsing logic is complex and could benefit from being split into smaller, focused functions for better maintainability.
Consider refactoring like this:
- const [baseHexcode, trailingLine] = line.split(";"); - // ... parsing logic ... - emojiMetadata[metadataGroup][hexcode] = { - group: groupName, - // ... metadata properties ... - }; + const parseMetadata = (line: string, currentGroup?: EmojiGroup): EmojiMetadata => { + const [baseHexcode, trailingLine] = line.split(";"); + // ... parsing logic ... + return { + group: groupName, + // ... metadata properties ... + }; + }; + + const metadata = parseMetadata(line, currentGroup); + emojiMetadata[metadataGroup][metadata.hexcode] = metadata;src/cli.ts (1)
26-42: Enhance command description for better clarity.The current description is quite brief. Consider adding more details about available generators and shortcode providers in the help text.
- "generate emoji data for the specified versions", + "generate emoji data for the specified versions using selected generators (metadata, sequences, variations, emojis, shortcodes) and shortcode providers (github, joypixels, iamcal)",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/adapter/base.ts(2 hunks)src/adapter/index.ts(3 hunks)src/cli.ts(3 hunks)src/schemas.ts(1 hunks)src/types.ts(2 hunks)
🔇 Additional comments (14)
src/schemas.ts (3)
3-7: LGTM! Well-defined shortcode provider schema.The schema correctly defines the supported shortcode providers using a union of literal values.
9-9: LGTM! Array schema for multiple providers.The array schema allows for flexible provider combinations while maintaining type safety.
11-18: LGTM! Comprehensive generator schema.The schema effectively covers all generator types using a union of literal values.
src/adapter/index.ts (3)
40-44: LGTM! New adapter capabilities added.The new properties enhance the adapter's functionality with shortcodes, metadata, and unicode names support.
47-50: LGTM! Improved version handling.Splitting version into
emojiVersionandunicodeVersionprovides better clarity and control.
97-102: LGTM! Custom error class for better error handling.The
MojisNotImplementederror class improves error identification and messaging.src/types.ts (2)
25-34: LGTM! Well-structured metadata interface.The
EmojiMetadatainterface provides a comprehensive structure for emoji metadata.
36-44: LGTM! Complete emoji data structure.The
EmojiDatainterface effectively captures all necessary emoji properties.src/adapter/base.ts (2)
17-24: LGTM! Proper version handling with early return.The function correctly handles unsupported emoji versions with a warning.
121-146: LGTM! Efficient unicode names parsing.The implementation correctly parses and caches unicode names with proper error handling.
src/cli.ts (4)
5-12: LGTM! Well-organized imports.The new imports are properly organized and necessary for the enhanced functionality.
151-155: Verify the purpose of empty emojis.json.An empty JSON object is being written to
emojis.json. Is this intentional or should it contain the emoji data?
77-97: Consider extracting common command logic to reduce duplication.The generator blocks share significant code patterns that could be extracted into a reusable function.
Also applies to: 99-119, 121-134, 136-156, 158-180
84-96: Add error handling for file operations.The file operations lack proper error handling.
Also applies to: 106-118, 128-133, 143-155, 171-179
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/adapter/base.ts (1)
18-117: Consider refactoring the metadata function for better readability.The metadata function is quite long and handles multiple responsibilities. Consider breaking it down into smaller, focused functions:
- Version validation
- Group/subgroup parsing
- Emoji metadata processing
Example refactor:
+function validateEmojiVersion(version: string): { groups: [], emojiMetadata: {} } | null { + if (version === "1.0" || version === "2.0" || version === "3.0") { + consola.warn(`skipping metadata for emoji version ${yellow(version)}, as it's not supported.`); + return { + groups: [], + emojiMetadata: {}, + }; + } + return null; +} +function parseGroupAndSubgroup(line: string, currentGroup?: EmojiGroup): { + group?: EmojiGroup, + subgroup?: string +} { + if (line.startsWith("# group:")) { + const groupName = line.slice(8).trim(); + return { + group: { + name: groupName, + slug: slugify(groupName), + subgroups: [], + } + }; + } else if (line.startsWith("# subgroup:")) { + const subgroupName = line.slice(11).trim(); + if (currentGroup == null) { + throw new Error(`subgroup ${subgroupName} without group`); + } + return { subgroup: slugify(subgroupName) }; + } + return {}; +} metadata: async (ctx) => { - if (ctx.emojiVersion === "1.0" || ctx.emojiVersion === "2.0" || ctx.emojiVersion === "3.0") { - consola.warn(`skipping metadata for emoji version ${yellow(ctx.emojiVersion)}, as it's not supported.`); - return { - groups: [], - emojiMetadata: {}, - }; + const skipResult = validateEmojiVersion(ctx.emojiVersion); + if (skipResult) { + return skipResult; } return fetchCache(`https://unicode.org/Public/emoji/${ctx.emojiVersion}/emoji-test.txt`, { cacheKey: `v${ctx.emojiVersion}/metadata.json`, parser(data) { const lines = data.split("\n"); let currentGroup: EmojiGroup | undefined; const groups: EmojiGroup[] = []; const emojiMetadata: Record<string, Record<string, EmojiMetadata>> = {}; for (const line of lines) { if (line.trim() === "") { continue; } - if (line.startsWith("# group:")) { - const groupName = line.slice(8).trim(); - const group: EmojiGroup = { - name: groupName, - slug: slugify(groupName), - subgroups: [], - }; - currentGroup = group; - groups.push(group); - continue; - } else if (line.startsWith("# subgroup:")) { - const subgroupName = line.slice(11).trim(); - if (currentGroup == null) { - throw new Error(`subgroup ${subgroupName} without group`); - } - currentGroup.subgroups.push(slugify(subgroupName)); - continue; + const { group, subgroup } = parseGroupAndSubgroup(line, currentGroup); + if (group) { + currentGroup = group; + groups.push(group); + continue; + } + if (subgroup) { + currentGroup!.subgroups.push(subgroup); + continue; }
📜 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 (5)
package.json(1 hunks)src/adapter/base.ts(2 hunks)src/cli.ts(5 hunks)src/utils.ts(4 hunks)src/utils/cache.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/cache.ts
🔇 Additional comments (10)
src/adapter/base.ts (2)
8-12: LGTM! Enhanced error handling with custom error class.The function now uses a custom error class with improved error message formatting.
122-147: LGTM! Well-structured unicode names implementation.The function effectively:
- Fetches unicode data with proper caching
- Parses the data line by line
- Handles error cases appropriately
- Returns a clean mapping of hex codes to names
src/cli.ts (4)
34-43: LGTM! Well-structured command options.The new options for generators and shortcode providers are well-defined with clear descriptions and sensible defaults.
78-181: Add error handling for file operations.The file operations in the generator blocks lack proper error handling.
Apply this diff to improve error handling:
if (isGeneratorEnabled("metadata")) { if (adapter.metadata == null) { throw new MojisNotImplemented("metadata"); } const { groups, emojiMetadata } = await adapter.metadata({ emojiVersion: version, force, unicodeVersion: getUnicodeVersionByEmojiVersion(version)! }); - await fs.ensureDir(`./data/v${version}/metadata`); + try { + await fs.ensureDir(`./data/v${version}/metadata`); - await fs.writeFile( - `./data/v${version}/groups.json`, - JSON.stringify(groups, null, 2), - "utf-8", - ); + await fs.writeFile( + `./data/v${version}/groups.json`, + JSON.stringify(groups, null, 2), + "utf-8", + ); - await Promise.all(Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( - `./data/v${version}/metadata/${group}.json`, - JSON.stringify(metadata, null, 2), - "utf-8", - ))); + await Promise.all(Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( + `./data/v${version}/metadata/${group}.json`, + JSON.stringify(metadata, null, 2), + "utf-8", + ))); + } catch (error) { + throw new Error(`Failed to write metadata files: ${error.message}`); + } }
200-226: LGTM! Enhanced logging with consola.The command now uses
consolafor improved logging output.
228-252: LGTM! Enhanced logging with consola.The command now uses
consolafor improved logging output.src/utils.ts (3)
284-292: Enhance version extraction robustness.The current implementation has limitations:
- The regex pattern is too restrictive:
- Does not handle patch versions (e.g., "E14.0.1")
- Does not handle single-digit versions (e.g., "E1.0")
- Missing input validation for null/undefined
Apply this diff to improve the implementation:
export function extractEmojiVersion(comment: string): string | null { + if (comment == null) { + return null; + } - const version = comment.match(/E(\d+\.\d)/); + const version = comment.match(/E(\d+\.\d+(?:\.\d+)?)/); if (version != null && version[1] != null) { return version[1].trim(); } return null; }
295-333: LGTM! Well-implemented unicode version extraction.The function:
- Correctly handles version coercion
- Has proper null checks
- Follows the Unicode specification for version mapping
- Has good error handling
335-358: Add documentation and handle missing case.The function needs the following improvements:
- Missing JSDoc documentation
- Error messages could be more descriptive
- Missing case "0.7" that exists in
extractUnicodeVersionApply this diff to improve the implementation:
+/** + * Gets the unicode version for a given emoji version. + * Throws an error if the emoji version is invalid. + * + * @param {string} emojiVersion - The emoji version to get unicode version for + * @returns {string} The corresponding unicode version + * @throws {Error} If the emoji version is invalid or unsupported + * + * @example + * ```ts + * getUnicodeVersionByEmojiVersion("11.0") // returns "11.0" + * getUnicodeVersionByEmojiVersion("5.0") // returns "10.0" + * getUnicodeVersionByEmojiVersion("invalid") // throws Error + * ``` + */ export function getUnicodeVersionByEmojiVersion(emojiVersion: string): string { const coercedEmojiVersion = semver.coerce(emojiVersion); if (coercedEmojiVersion == null) { - throw new Error(`invalid emoji version: ${emojiVersion}`); + throw new Error(`Invalid emoji version format: ${emojiVersion}. Expected format: major.minor[.patch]`); } if (semver.gte(coercedEmojiVersion, "11.0.0")) { return emojiVersion; } switch (emojiVersion) { + case "0.7": + return "7.0"; case "1.0": case "2.0": return "8.0"; case "3.0": case "4.0": return "9.0"; case "5.0": return "10.0"; default: - throw new Error(`invalid emoji version: ${emojiVersion}`); + throw new Error(`Unsupported emoji version: ${emojiVersion}. Only versions 0.7 through 5.0 and 11.0+ are supported.`); } }package.json (1)
37-37: LGTM! Added consola for improved logging.The addition of the
consolapackage enhances the logging capabilities across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/shortcode/github.ts (1)
30-35: Strengthen the regex pattern for URL parsing.The current regex pattern might be fragile if GitHub changes their URL structure. Consider:
- Making the pattern more specific to avoid false matches
- Adding a test case for URL format changes
- const match = url.match(/emoji\/unicode\/([\da-z-]+)\.png/i); + const match = url.match(/^https:\/\/github\.githubassets\.com\/images\/icons\/emoji\/unicode\/([\da-z-]+)\.png$/i);src/adapter/index.ts (1)
86-88: Address TODO comment about adapter validation.The TODO comment indicates missing validation for required functions when not extending another adapter.
Would you like me to help implement the validation logic for required adapter functions? This would ensure that adapters that don't extend others have all necessary functions implemented.
src/adapter/base.ts (1)
147-173: Consider adding support for more shortcode providers.The
shortcodesfunction is well-implemented but currently only supports GitHub shortcodes. Consider adding support for other popular providers like Slack, Discord, etc.Do you want me to help implement support for additional shortcode providers?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/adapter/base.ts(2 hunks)src/adapter/index.ts(3 hunks)src/adapter/v16.ts(4 hunks)src/cli.ts(5 hunks)src/schemas.ts(1 hunks)src/shortcode/github.ts(1 hunks)src/types.ts(2 hunks)src/utils/hexcode.ts(1 hunks)test/utils/hexcode.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/schemas.ts
🧰 Additional context used
🪛 GitHub Check: build
src/adapter/v16.ts
[failure] 1-1:
'EmojiShortcode' is defined but never used
[failure] 1-1:
'ShortcodeProvider' is defined but never used
[failure] 74-74:
'sequences' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 74-74:
'zwj' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 75-75:
'metadata' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 76-76:
'variations' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 78-78:
'emojis' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 144-144:
'hex' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 144-144:
'data' is assigned a value but never used. Allowed unused vars must match /^_/u
src/adapter/index.ts
[failure] 1-1:
'EmojiData' is defined but never used
🪛 ESLint
src/adapter/v16.ts
[error] 1-1: 'EmojiShortcode' is defined but never used.
(unused-imports/no-unused-imports)
[error] 1-1: 'ShortcodeProvider' is defined but never used.
(unused-imports/no-unused-imports)
[error] 74-74: 'sequences' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 74-74: 'zwj' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 75-75: 'metadata' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 76-76: 'variations' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 78-78: 'emojis' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 144-144: 'hex' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 144-144: 'data' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 144-146: Empty block statement.
(no-empty)
src/adapter/index.ts
[error] 1-1: 'EmojiData' is defined but never used.
(unused-imports/no-unused-imports)
🪛 GitHub Actions: CI
src/adapter/index.ts
[error] 1-1: 'EmojiData' is defined but never used
🔇 Additional comments (14)
src/utils/hexcode.ts (1)
56-68: LGTM! Well-documented and implemented.The
stripHexfunction is:
- Well-documented with clear JSDoc comments
- Correctly handles both hyphen and space separators
- Uses an appropriate regex pattern for the specified variation selectors
test/utils/hexcode.test.ts (1)
40-64: LGTM! Comprehensive test coverage.The test suite:
- Covers all variation selectors (200D, FE0E, FE0F)
- Includes edge cases
- Tests multiple separator formats
src/types.ts (4)
10-16: LGTM!The changes to the
Emojiinterface improve flexibility by supporting multiple hex codes and shortcodes, making it more suitable for handling complex emoji data.
18-18: LGTM!The
ShortcodeProvidertype is well-defined using type inference from the schema, ensuring type safety for shortcode provider validation.
20-29: LGTM!The
EmojiMetadatainterface is well-structured with comprehensive fields for emoji metadata, using appropriate types for each field.
31-39: LGTM!The
EmojiDatainterface is well-structured with comprehensive fields for emoji data, using appropriate types for each field.src/adapter/base.ts (2)
18-117: LGTM!The
metadatafunction is well-implemented with proper handling of unsupported versions, caching, and thorough parsing logic.
121-146: LGTM!The
unicodeNamesfunction is well-implemented with proper caching and thorough parsing logic for Unicode data.src/adapter/v16.ts (2)
13-71: LGTM!The
sequencesfunction has been correctly updated to use the new context parameter while maintaining its thorough parsing logic.
150-195: LGTM!The
variationsfunction has been correctly updated to use the new context parameter while maintaining its thorough parsing logic.src/cli.ts (4)
26-197: Extract common command logic to reduce duplication.Consider extracting the common logic into a reusable function to improve maintainability.
85-97: Add error handling for file operations.The file operations lack proper error handling. Consider wrapping them in try-catch blocks.
Also applies to: 107-119, 129-134, 144-150, 166-179
199-225: LGTM!The command has been correctly updated to use the new logging mechanism while maintaining its thorough logic.
227-251: LGTM!The command has been correctly updated to use the new logging mechanism while maintaining its thorough logic.
| export interface ShortcodeOptions { | ||
| version: string; | ||
| force: boolean; | ||
| emojis: any; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for ShortcodeOptions interface.
The emojis property is typed as any, which reduces type safety. Consider defining a more specific type.
export interface ShortcodeOptions {
version: string;
force: boolean;
- emojis: any;
+ emojis: Record<string, unknown>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface ShortcodeOptions { | |
| version: string; | |
| force: boolean; | |
| emojis: any; | |
| } | |
| export interface ShortcodeOptions { | |
| version: string; | |
| force: boolean; | |
| emojis: Record<string, unknown>; | |
| } |
| const githubEmojis = await fetchCache<Record<string, string>>("https://api.github.com/emojis", { | ||
| cacheKey: `v${version}/github-emojis.json`, | ||
| bypassCache: force, | ||
| parser(data) { | ||
| return JSON.parse(data); | ||
| }, | ||
| options: { | ||
| headers: { | ||
| "Accept": "application/vnd.github.v3+json", | ||
| "User-Agent": "mojis.dev", | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider GitHub API rate limiting.
The GitHub API has rate limits. Consider:
- Adding error handling for rate limit responses
- Including rate limit headers in the cache metadata
Let me verify the GitHub API rate limits and authentication requirements:
🌐 Web query:
What are the rate limits for GitHub's public API endpoints?
💡 Result:
GitHub's public API enforces several rate limits to ensure reliability and prevent abuse across both REST and GraphQL endpoints. These limits vary by authentication method and API type:
Primary Rate Limits
REST API
- Unauthenticated: 60 requests/hour (IP-based) [7][9].
- Authenticated users:
- Standard: 5,000 requests/hour (personal access tokens/OAuth apps) [7][9].
- GitHub Enterprise Cloud members: Up to 15,000 requests/hour via approved OAuth apps or GitHub Apps [1][3].
- GitHub Apps:
- Non-Enterprise: Starts at 5,000 requests/hour, scaling up with repositories/users (max 12,500) [1][9].
- Enterprise Cloud: 15,000 requests/hour [3][4].
GraphQL API
Uses a points system instead of request counts:
- Authenticated users:
- Standard: 5,000 points/hour [1][8].
- Enterprise Cloud: 10,000 points/hour [1].
- Points per query:
- Non-mutation: 1 point
- Mutation: 5 points [1][3].
Secondary Rate Limits
Applies to both APIs:
| Limit Type | Threshold |
|---|---|
| Concurrent requests | ≤100 across both APIs [1][10] |
| Points/minute | ≤900 (REST), ≤2000 (GraphQL) [1][3] |
| CPU time | ≤90s/60s real time (≤60s for GraphQL) [1][10] |
| Content creation | ≤80/min and ≤500/hour [9][10] |
Special Cases
GITHUB_TOKENin Actions:- Standard repos: 1,000 requests/hour
- Enterprise Cloud: 15,000 requests/hour [3][7].
- Search API: Stricter limits than general REST endpoints [9].
Monitoring Tools
Response headers include:
x-ratelimit-limit: Total allowed points/requestsx-ratelimit-remaining: Available quota [1][2]x-ratelimit-reset: UTC epoch reset time [2][8]
For optimization:
- Use conditional requests with
ETagheaders to avoid redundant calls [7]. - Maximize page sizes (up to 100 items/page) [7].
- Implement exponential backoff for retries after
429errors [2][3].
Exceeding limits triggers 403 Forbidden or 429 Too Many Requests errors with retry-after headers indicating wait times[2][10].
Citations:
- 1: https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api
- 2: https://docs.github.com/en/rest/using-the-rest-api/troubleshooting-the-rest-api
- 3: https://www.lunar.dev/post/a-developers-guide-managing-rate-limits-for-the-github-api
- 4: https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/rate-limits-for-github-apps
- 5: https://github.blog/changelog/2023-10-18-rate-limits-for-rate_limit-rest-api-endpoint/
- 6: https://dev.to/codexam/github-api-rate-limit-exceeded-problem-502f
- 7: https://www.endorlabs.com/learn/how-to-get-the-most-out-of-github-api-rate-limits
- 8: https://jamiemagee.co.uk/blog/making-the-most-of-github-rate-limits/
- 9: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
- 10: https://docs.github.com/en/[email protected]/rest/using-the-rest-api/rate-limits-for-the-rest-api
Action Required: Implement Rate Limit Handling for GitHub API
After verifying GitHub's public API rate limits, please update the code to gracefully handle rate-limit scenarios. Specifically, consider the following:
- Error Handling for Rate Limits: Add logic to detect and handle error responses (e.g., 403 Forbidden or 429 Too Many Requests) when rate limits are exceeded. Implement exponential backoff or retry logic based on the
retry-afterheader. - Rate Limit Metadata: Capture and cache rate limit headers (such as
x-ratelimit-limit,x-ratelimit-remaining, andx-ratelimit-reset) alongside your cached data to aid in debugging and monitoring.
These updates will help ensure that your implementation handles GitHub’s rate limits—60 requests/hr for unauthenticated users and higher limits when authenticated—as documented.
| @@ -1,4 +1,4 @@ | |||
| import type { EmojiGroup, EmojiSequence, EmojiVariation } from "../types"; | |||
| import type { Emoji, EmojiData, EmojiGroup, EmojiMetadata, EmojiSequence, EmojiShortcode, EmojiVariation, ShortcodeProvider } from "../types"; | |||
There was a problem hiding this comment.
Remove unused import.
The EmojiData type is imported but never used, as flagged by static analysis.
-import type { Emoji, EmojiData, EmojiGroup, EmojiMetadata, EmojiSequence, EmojiShortcode, EmojiVariation, ShortcodeProvider } from "../types";
+import type { Emoji, EmojiGroup, EmojiMetadata, EmojiSequence, EmojiShortcode, EmojiVariation, ShortcodeProvider } from "../types";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { Emoji, EmojiData, EmojiGroup, EmojiMetadata, EmojiSequence, EmojiShortcode, EmojiVariation, ShortcodeProvider } from "../types"; | |
| import type { Emoji, EmojiGroup, EmojiMetadata, EmojiSequence, EmojiShortcode, EmojiVariation, ShortcodeProvider } from "../types"; |
🧰 Tools
🪛 GitHub Check: build
[failure] 1-1:
'EmojiData' is defined but never used
🪛 ESLint
[error] 1-1: 'EmojiData' is defined but never used.
(unused-imports/no-unused-imports)
🪛 GitHub Actions: CI
[error] 1-1: 'EmojiData' is defined but never used
src/adapter/v16.ts
Outdated
| async emojis(ctx) { | ||
| const unicodeNames = await this.unicodeNames!(ctx); | ||
| const { sequences, zwj } = await this.sequences!(ctx); | ||
| const metadata = await this.metadata!(ctx); | ||
| const variations = await this.variations!(ctx); | ||
|
|
||
| const emojis: Record<string, Emoji> = {}; | ||
|
|
||
| const emojiData = await fetchCache(`https://unicode.org/Public/${ctx.emojiVersion}.0/ucd/emoji/emoji-data.txt`, { | ||
| cacheKey: `v${ctx.emojiVersion}/emoji-data.json`, | ||
| parser(data) { | ||
| const lines = data.split("\n"); | ||
|
|
||
| const emojiData: Record<string, EmojiData> = {}; | ||
|
|
||
| for (const line of lines) { | ||
| // skip empty line & comments | ||
| if (line.trim() === "" || line.startsWith("#")) { | ||
| continue; | ||
| } | ||
|
|
||
| const lineCommentIndex = line.indexOf("#"); | ||
| const lineComment = lineCommentIndex !== -1 ? line.slice(lineCommentIndex + 1).trim() : ""; | ||
|
|
||
| let [hex, property] = line.split(";").map((col) => col.trim()).slice(0, 4); | ||
|
|
||
| if (hex == null || property == null) { | ||
| throw new Error(`invalid line: ${line}`); | ||
| } | ||
|
|
||
| // remove line comment from property | ||
| const propertyCommentIndex = property.indexOf("#"); | ||
| if (propertyCommentIndex !== -1) { | ||
| property = property.slice(0, propertyCommentIndex).trim(); | ||
| } | ||
|
|
||
| if (property === "Extended_Pictographic") { | ||
| continue; | ||
| } | ||
|
|
||
| const expandedHex = expandHexRange(hex); | ||
| const emojiVersion = extractEmojiVersion(lineComment); | ||
|
|
||
| const emoji: EmojiData = { | ||
| description: lineComment, | ||
| hexcode: "", | ||
| gender: null, | ||
| properties: [(property as Property) || "Emoji"], | ||
| unicodeVersion: extractUnicodeVersion(emojiVersion, ctx.unicodeVersion), | ||
| emojiVersion, | ||
| name: unicodeNames[hex] || "", | ||
| }; | ||
|
|
||
| for (const hex of expandedHex) { | ||
| if (emojiData[hex] != null) { | ||
| emojiData[hex].properties = [...new Set([...emojiData[hex].properties, ...emoji.properties])]; | ||
| } else { | ||
| emojiData[hex] = { | ||
| ...emoji, | ||
| hexcode: hex.replace(/\s+/g, "-"), | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return emojiData; | ||
| }, | ||
| bypassCache: ctx.force, | ||
| }); | ||
|
|
||
| // join names, metadata, variations, sequences, zwj | ||
|
|
||
| for (const [hex, data] of Object.entries(emojiData)) { | ||
|
|
||
| } | ||
|
|
||
| return {}; | ||
| }, |
There was a problem hiding this comment.
Complete the implementation of the emojis function.
The function has several issues:
- The empty block at lines 144-146 needs to be implemented.
- Several variables are fetched but never used:
sequences,zwj,metadata,variations,emojis,hex,data.
Please complete the implementation by using the fetched data to construct and return the emoji objects.
🧰 Tools
🪛 GitHub Check: build
[failure] 74-74:
'sequences' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 74-74:
'zwj' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 75-75:
'metadata' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 76-76:
'variations' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 78-78:
'emojis' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 144-144:
'hex' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 144-144:
'data' is assigned a value but never used. Allowed unused vars must match /^_/u
🪛 ESLint
[error] 74-74: 'sequences' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 74-74: 'zwj' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 75-75: 'metadata' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 76-76: 'variations' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 78-78: 'emojis' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 144-144: 'hex' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 144-144: 'data' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
[error] 144-146: Empty block statement.
(no-empty)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/cli.ts (1)
78-167: 🛠️ Refactor suggestionAdd error handling for file operations.
Consider wrapping file operations in try-catch blocks to handle potential file system errors gracefully.
+ async function writeJsonFile(path: string, data: unknown) { + try { + await fs.ensureDir(path.substring(0, path.lastIndexOf("/"))); + await fs.writeFile(path, JSON.stringify(data, null, 2), "utf-8"); + } catch (error) { + throw new Error(`Failed to write file ${path}: ${error.message}`); + } + } if (isGeneratorEnabled("metadata")) { if (adapter.metadata == null) { throw new MojisNotImplemented("metadata"); } const { groups, emojiMetadata } = await adapter.metadata({ emojiVersion: version, force, unicodeVersion: getUnicodeVersionByEmojiVersion(version)! }); - await fs.ensureDir(`./data/v${version}/metadata`); - await fs.writeFile( - `./data/v${version}/groups.json`, - JSON.stringify(groups, null, 2), - "utf-8", - ); + await writeJsonFile(`./data/v${version}/groups.json`, groups); - await Promise.all(Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( - `./data/v${version}/metadata/${group}.json`, - JSON.stringify(metadata, null, 2), - "utf-8", - ))); + await Promise.all(Object.entries(emojiMetadata).map(([group, metadata]) => + writeJsonFile(`./data/v${version}/metadata/${group}.json`, metadata) + )); }
🧹 Nitpick comments (12)
src/adapter/v14.ts (3)
13-71: Consider extracting shared parsing logic for maintainability.
The code segment for parsing emoji sequence lines (removing comments, expanding hex ranges, etc.) appears in multiple adapter versions. refactoring this logic into a shared utility function could reduce duplication and improve maintainability.
72-146: Verify usage of the empty 'emojis' object.
Though this method returns bothemojiDataand anemojisobject, theemojisrecord remains unused. If this is a placeholder for future logic, consider adding a TODO comment; otherwise, remove it to avoid confusion.
147-192: DRY opportunity in variation parsing.
The logic for removing comments, splitting lines, and expanding hex codes mirrors the approach taken insequences. Refactoring repeated parser code into a helper utility would streamline this file and reduce duplication across adapters.src/adapter/v15.ts (3)
13-71: Extract parser logic to reduce duplication.
As in the other adapters, thesequencesmethod implements line-based parsing with repeated patterns (e.g., ignoring comments and empty lines, expanding hex codes). Consider unifying this logic in a shared helper function.
72-146: 'emojis' object remains empty—verify necessity.
The method returnsemojiDataandemojis, but the latter is never assigned any values. Confirm if this is a placeholder for upcoming features or if it should be removed.
147-192: Repeated variation parsing suggests modular approach.
The code filtering lines, trimming comments, and handling style checks is identical tosequencesin structure. Extracting this repeated logic into a shared utility would align with DRY principles.src/adapter/v13.ts (3)
13-71: Shared parser logic could be extracted for clarity.
The loop for parsing lines and creatingEmojiSequenceinstances has the same structure as inv14andv15. Centralizing this parser logic would help maintain consistency and simplify changes in the future.
72-146: Confirm fallback for 13.1 fetching from 13.0.
This method fetches from13.0whenctx.emojiVersionis13.1. Verify this is valid per Unicode data distribution. Theemojisobject is also returned but remains empty, so confirm whether it will be populated later or removed.
147-192: Variation parsing approach is consistent but repetitive.
The logic closely matches that of thesequencesmethod. Extracting the common parsing steps into a helper would reduce duplication across the codebase.src/adapter/base.ts (2)
18-116: Add error handling for data parsing.While the parsing logic is thorough, consider adding try-catch blocks around the data parsing to handle potential errors gracefully.
parser(data) { + try { const lines = data.split("\n"); let currentGroup: EmojiGroup | undefined; const groups: EmojiGroup[] = []; const emojiMetadata: Record<string, Record<string, EmojiMetadata>> = {}; for (const line of lines) { // ... existing parsing logic ... } return { groups, emojiMetadata, }; + } catch (error) { + throw new Error(`Failed to parse metadata: ${error.message}`); + } }
147-183: Optimize emoji flattening.The current implementation of flattening emojis uses nested loops and object operations. Consider using
Object.entrieswithreducefor better performance.- const flattenedEmojis = Object.values(emojis).reduce((acc, subgroup) => { - for (const hexcodes of Object.values(subgroup)) { - for (const [hexcode, emoji] of Object.entries(hexcodes)) { - acc[hexcode] = emoji; - } - } - return acc; - }, {} as Record<string, Emoji>); + const flattenedEmojis = Object.entries(emojis).reduce((acc, [, subgroups]) => + Object.entries(subgroups).reduce((subAcc, [, hexcodes]) => + Object.entries(hexcodes).reduce((hexAcc, [hexcode, emoji]) => { + hexAcc[hexcode] = emoji; + return hexAcc; + }, subAcc), acc), {} as Record<string, Emoji>);src/adapter/v16.ts (1)
72-145: Improve property merging logic.The current implementation of property merging uses spread operator with Set. Consider using
Array.fromwith Set for better readability.- emojiData[hex].properties = [...new Set([...emojiData[hex].properties, ...emoji.properties])]; + emojiData[hex].properties = Array.from(new Set([...emojiData[hex].properties, ...emoji.properties]));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/adapter/base.ts(2 hunks)src/adapter/index.ts(3 hunks)src/adapter/v13.ts(1 hunks)src/adapter/v14.ts(1 hunks)src/adapter/v15.ts(1 hunks)src/adapter/v16.ts(4 hunks)src/cli.ts(5 hunks)
🔇 Additional comments (14)
src/adapter/v14.ts (1)
1-6: Imports look consistent and necessary.
No issues found in these import statements; they correctly pull in the modules and types used throughout the adapter.src/adapter/v15.ts (1)
1-1: Imports are appropriate, but watch for future code sharing.
All imported constants and utilities are used consistently below. If you find that identical imports are maintained across multiple adapters, consider grouping them in a shared module.Also applies to: 3-6
src/adapter/v13.ts (1)
1-6: Imports align with usage; no immediate issues noted.
All imported references (types, constants, and utilities) are necessary for the logic below and match the usage in other adapters.src/adapter/index.ts (5)
1-1: LGTM!The
EmojiDataimport is now used in theEmojiFntype definition.
40-44: LGTM!The new optional methods are well-typed and follow the existing pattern of optional adapter methods.
48-49: LGTM!The property names are more descriptive and provide better separation between emoji and unicode versions.
53-67: LGTM!The type definitions are well-structured and provide comprehensive type safety for the adapter methods.
99-104: LGTM!The error class follows best practices for custom error types.
src/adapter/base.ts (1)
1-5: LGTM!The imports are well-organized and used in the updated code.
src/adapter/v16.ts (2)
14-22: LGTM!The function correctly uses the new
emojiVersionproperty from the context.
148-150: LGTM!The function correctly uses the new
emojiVersionproperty from the context.src/cli.ts (3)
27-43: LGTM!The command structure is well-organized with properly typed options and sensible defaults.
169-196: LGTM!The shortcode provider handling is thorough with proper validation and error handling.
201-211: LGTM!The error handling is well-structured with appropriate logging for different error types.
… and simplify sorting logic
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/utils.ts (2)
288-309: 🛠️ Refactor suggestionEnhance version extraction robustness.
The current implementation has the following limitations:
- The regex pattern is too restrictive:
- Does not handle patch versions (e.g., "E14.0.1")
- Does not handle single-digit versions (e.g., "E1.0")
- Missing input validation for null/undefined
Apply this diff to improve the implementation:
export function extractEmojiVersion(comment: string): string | null { + if (comment == null) { + return null; + } - const version = comment.match(/E(\d+\.\d)/); + const version = comment.match(/E(\d+\.\d+(?:\.\d+)?)/); if (version != null && version[1] != null) { return version[1].trim(); } return null; }
352-375: 🛠️ Refactor suggestionAdd documentation and handle missing case.
The function needs the following improvements:
- Missing JSDoc documentation
- Error messages could be more descriptive
- Missing case "0.7" that exists in
extractUnicodeVersionApply this diff to improve the implementation:
+/** + * Gets the unicode version for a given emoji version. + * Throws an error if the emoji version is invalid. + * + * @param {string} emojiVersion - The emoji version to get unicode version for + * @returns {string} The corresponding unicode version + * @throws {Error} If the emoji version is invalid or unsupported + * + * @example + * ```ts + * getUnicodeVersionByEmojiVersion("11.0") // returns "11.0" + * getUnicodeVersionByEmojiVersion("5.0") // returns "10.0" + * getUnicodeVersionByEmojiVersion("invalid") // throws Error + * ``` + */ export function getUnicodeVersionByEmojiVersion(emojiVersion: string): string { const coercedEmojiVersion = semver.coerce(emojiVersion); if (coercedEmojiVersion == null) { - throw new Error(`invalid emoji version: ${emojiVersion}`); + throw new Error(`Invalid emoji version format: ${emojiVersion}. Expected format: major.minor[.patch]`); } if (semver.gte(coercedEmojiVersion, "11.0.0")) { return emojiVersion; } switch (emojiVersion) { + case "0.7": + return "7.0"; case "1.0": case "2.0": return "8.0"; case "3.0": case "4.0": return "9.0"; case "5.0": return "10.0"; default: - throw new Error(`invalid emoji version: ${emojiVersion}`); + throw new Error(`Unsupported emoji version: ${emojiVersion}. Only versions 0.7 through 5.0 and 11.0+ are supported.`); } }
🧹 Nitpick comments (1)
src/utils.ts (1)
311-350: Add JSDoc documentation.The function needs proper documentation to explain its purpose, parameters, return value, and provide usage examples.
Add this documentation:
+/** + * Maps emoji versions to their corresponding unicode versions based on Unicode standards. + * For versions 11.0 and above, emoji versions are aligned with unicode versions. + * For earlier versions, specific mappings are applied according to Unicode TR#51. + * + * @see {@link https://unicode.org/reports/tr51/#EmojiVersions} + * + * @param {string | null} emojiVersion - The emoji version to map + * @param {string} [unicodeVersion] - Optional unicode version for comparison + * @returns {string | null} The corresponding unicode version or null if versions are invalid + * + * @example + * ```ts + * extractUnicodeVersion("14.0") // returns "14.0" + * extractUnicodeVersion("5.0") // returns "10.0" + * extractUnicodeVersion("1.0") // returns "8.0" + * ``` + */ export function extractUnicodeVersion(emojiVersion: string | null, unicodeVersion?: string): string | null {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils.ts(6 hunks)
🔇 Additional comments (1)
src/utils.ts (1)
121-154: LGTM! Improved version mapping and sorting logic.The changes correctly implement the emoji to unicode version mapping according to Unicode standards, and the sorting logic has been simplified for better readability.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli.ts (1)
34-38: Validate the elements of the 'generators' arrayConsider enforcing that the array only contains known generator names:
.option("generators", { type: "array", default: ["metadata", "sequences", "variations", "emojis", "shortcodes"], + coerce: (vals) => { + const known = new Set(["metadata","sequences","variations","emojis","shortcodes"]); + vals.forEach(val => { + if (!known.has(val)) { + throw new Error(`Unknown generator: ${val}`); + } + }); + return vals; + }, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cli.ts(5 hunks)src/utils.ts(6 hunks)test/lockfile.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/lockfile.test.ts
🧰 Additional context used
🪛 GitHub Check: build
src/cli.ts
[failure] 14-14:
Expected "./lockfile" to come before "./utils"
🪛 ESLint
src/cli.ts
[error] 14-14: Expected "./lockfile" to come before "./utils".
(perfectionist/sort-imports)
🪛 GitHub Actions: CI
src/cli.ts
[error] 14-14: Expected "./lockfile" to come before "./utils" perfectionist/sort-imports
🔇 Additional comments (26)
src/cli.ts (20)
2-2: Use ofconsola
Standardizing logs withconsolais a great approach for consistent message formatting across the CLI.
6-6: Valibot import
ImportingparseAsyncfrom "valibot" aligns with its usage below, ensuring type-safe parsing.
9-9: New custom error import
IncludingMojisNotImplementedhelps handle unimplemented generator logic cleanly.
12-12: Schema import
ImportingSHORTCODE_PROVIDERS_SCHEMAkeeps CLI options consistent with validation rules in the schema.
27-28: Command name change
Renaming the command to"generate <versions...>"is more concise and descriptive.
39-43: Shortcode providers option
Supplying a default of["github"]via the schema is consistent and ensures valid provider inputs.
48-52: Control flow clarity
Defining and invokingisGeneratorEnabled()yields clear, easy-to-read branching for generator logic.
54-59: Immediate exit on unsupported versions
Filtering and exiting on invalid versions prevents processing partial or incompatible data.
62-64: Progress logging
Usingconsola.infoto communicate generation details aligns with the standardized logging approach.
78-81: Validating 'metadata' generator
Cleanly throwsMojisNotImplementedif an adapter lacks ametadatamethod, preventing half-finished outputs.
83-97: Add error handling for file operations
100-120: Add error handling for file operations
122-135: Add error handling for file operations
137-167: Add error handling for file operations
169-196: Add error handling for file operations
203-207: Graceful fallback for unimplemented features
When a generator method is not found, continuing with other tasks ensures partial data can still be generated.
211-211: Completion message
Ending the process with a clear"done"message is user-friendly and transparent.
238-238: Consistent logging
Confirming lockfile updates withconsola.logmaintains uniformity with the rest of the CLI.
255-256: Informative listing of versions
Logging all available versions at once helps users see the full coverage.
264-264: Lockfile update indication
Announcing the updated lockfile fosters clarity on changes persisted to disk.src/utils.ts (6)
1-2: Imports of lockfile type andconsola
Bringing inEmojiVersionfrom"./lockfile"and usingconsolafor logging aligns with the broader architectural approach.
64-68: Error reporting withconsola.error
The standardized error logging improves visibility and debugging.
186-186: Strict check for legacy major versions
Ensuring minor/patch are zero for<= 5.xstrictly follows older emoji versioning rules.
288-309: Enhance version extraction for patch versions
311-350: Comprehensive mapping of emoji-to-unicode versions
This alignment logic for 0.7 through 5.0 and beyond is coherent with documented specs.
352-375: Missing case for 0.7
src/cli.ts
Outdated
| import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils"; | ||
| import { readLockfile, writeLockfile } from "./lockfile"; |
There was a problem hiding this comment.
Fix import sorting to satisfy ESLint
The CI pipeline fails due to the import order. Reorder the imports to place "./lockfile" before "./utils", as required by the perfectionist/sort-imports rule:
- import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils";
- import { readLockfile, writeLockfile } from "./lockfile";
+ import { readLockfile, writeLockfile } from "./lockfile";
+ import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils"; | |
| import { readLockfile, writeLockfile } from "./lockfile"; | |
| import { readLockfile, writeLockfile } from "./lockfile"; | |
| import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils"; |
🧰 Tools
🪛 GitHub Check: build
[failure] 14-14:
Expected "./lockfile" to come before "./utils"
🪛 ESLint
[error] 14-14: Expected "./lockfile" to come before "./utils".
(perfectionist/sort-imports)
🪛 GitHub Actions: CI
[error] 14-14: Expected "./lockfile" to come before "./utils" perfectionist/sort-imports
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/versions.test.ts (1)
66-125: Add tests for patch versions if applicable.You test invalid and valid major.minor scenarios in
extractVersionFromReadmeandextractEmojiVersion. If patch versions (likeE15.1.1) or more nuanced formats ever become relevant, consider adding similar tests to maintain coverage.src/cli.ts (2)
152-166: Optimize nested loops for emoji data generation.The triple-nested loops for writing emoji data could lead to performance issues with a large number of emojis.
Consider batching the file operations:
- for (const [group, subgroup] of Object.entries(emojis)) { - await fs.ensureDir(`./data/v${version}/emojis/${group}`); - - for (const hexcodes of Object.values(subgroup)) { - await fs.ensureDir(`./data/v${version}/emojis/${group}/${subgroup}`); - - for (const [hexcode, emoji] of Object.entries(hexcodes)) { - await fs.writeFile( - `./data/v${version}/emojis/${group}/${subgroup}/${hexcode}.json`, - JSON.stringify(emoji, null, 2), - "utf-8", - ); - } - } - } + const writeOperations = []; + for (const [group, subgroup] of Object.entries(emojis)) { + writeOperations.push(fs.ensureDir(`./data/v${version}/emojis/${group}`)); + for (const hexcodes of Object.values(subgroup)) { + writeOperations.push(fs.ensureDir(`./data/v${version}/emojis/${group}/${subgroup}`)); + for (const [hexcode, emoji] of Object.entries(hexcodes)) { + writeOperations.push(fs.writeFile( + `./data/v${version}/emojis/${group}/${subgroup}/${hexcode}.json`, + JSON.stringify(emoji, null, 2), + "utf-8", + )); + } + } + } + await Promise.all(writeOperations);
169-196: Enhance error handling for shortcode providers.The error handling for shortcode providers could be improved to provide more detailed error messages.
Consider adding more specific error messages:
if (providers.length === 0) { - throw new Error("no shortcode providers specified"); + throw new Error("No shortcode providers specified. Please provide at least one provider using --shortcode-providers option."); } if (adapter.shortcodes == null) { - throw new MojisNotImplemented("shortcodes"); + throw new MojisNotImplemented(`Shortcode generation is not implemented for emoji version ${version}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/cli.ts(5 hunks)src/utils.ts(6 hunks)src/versions.test.ts(1 hunks)src/versions.ts(1 hunks)test/fixtures/extract-version/emoji/README-invalid.txt(1 hunks)test/fixtures/extract-version/emoji/README-valid.txt(1 hunks)test/fixtures/extract-version/root/README-invalid.txt(1 hunks)test/fixtures/extract-version/root/README-valid.txt(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- test/fixtures/extract-version/root/README-valid.txt
- test/fixtures/extract-version/root/README-invalid.txt
- test/fixtures/extract-version/emoji/README-invalid.txt
- test/fixtures/extract-version/emoji/README-valid.txt
🧰 Additional context used
🪛 GitHub Check: build
src/cli.ts
[failure] 14-14:
Module '"./utils"' has no exported member 'getUnicodeVersionByEmojiVersion'.
🪛 GitHub Actions: CI
src/cli.ts
[error] 14-14: Module './utils' has no exported member 'getUnicodeVersionByEmojiVersion'.
🔇 Additional comments (11)
src/versions.ts (3)
24-73: Great concurrency approach ingetCurrentDraftVersion.Fetching the root and emoji ReadMe files using
Promise.allSettledand then thoroughly checking each success/failure scenario is a robust pattern. Logging errors viaconsola.erroralso provides good visibility for debugging. No major issues found here.
88-96: Support patch versions if needed.This function only captures major.minor (e.g.,
E14.0) and ignores possible patch elements. This limitation was raised previously.You can expand the regex as follows:
- const version = comment.match(/E(\d+\.\d)/); + const version = comment.match(/E(\d+\.\d+(?:\.\d+)?)/);
154-169: Handle the missing case for"0.7"explicitly.A past review pointed out that
"0.7"may also need mapping to"7.0"or similar. Relying on the default"6.0"fallback might not reflect the actual version progression.Consider adding a case in the switch:
switch (emojiVersion) { + case "0.7": + return "7.0"; case "1.0": case "2.0": return "8.0"; ... }src/versions.test.ts (1)
10-26: Well-structured test for matching draft versions.Your mocks simulate successful fetch responses, and the resulting assertion comprehensively ensures that both emoji and Unicode versions are handled. Good coverage here.
src/utils.ts (3)
80-82: Good defensive check for null draft version.Throwing an error if
draftis null immediately halts further processing. This prevents silent failures and fosters clearer debugging.
126-159: Map the"0.7"emoji version to the correct Unicode version.Several other legacy emoji versions are mapped (e.g.,
"1.0"→"8.0.0"). If"0.7"is also valid in your project context, consider adding an explicit clause for consistency.if (match[1] === "0.7") { unicode_version = "7.0.0"; }
191-193: Clarify handling of"0.7"inisEmojiVersionValid.Because
semver.major("0.7")is0, this check declares it invalid. If you want"0.7"to pass, you'll need special handling similar to versions1.0…5.0.src/cli.ts (4)
12-14: Fix import sorting to satisfy ESLint.The import order needs to be adjusted to comply with ESLint rules.
Apply this diff to fix the import order:
- import { readLockfile, writeLockfile } from "./lockfile"; - import { SHORTCODE_PROVIDERS_SCHEMA } from "./schemas"; - import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils"; + import { readLockfile, writeLockfile } from "./lockfile"; + import { SHORTCODE_PROVIDERS_SCHEMA } from "./schemas"; + import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils";🧰 Tools
🪛 GitHub Check: build
[failure] 14-14:
Module '"./utils"' has no exported member 'getUnicodeVersionByEmojiVersion'.🪛 GitHub Actions: CI
[error] 14-14: Module './utils' has no exported member 'getUnicodeVersionByEmojiVersion'.
85-97: Add error handling for file operations.The metadata file operations lack proper error handling.
Apply this diff to add error handling:
- await fs.ensureDir(`./data/v${version}/metadata`); - - await fs.writeFile( - `./data/v${version}/groups.json`, - JSON.stringify(groups, null, 2), - "utf-8", - ); - - await Promise.all(Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( - `./data/v${version}/metadata/${group}.json`, - JSON.stringify(metadata, null, 2), - "utf-8", - ))); + try { + await fs.ensureDir(`./data/v${version}/metadata`); + + await fs.writeFile( + `./data/v${version}/groups.json`, + JSON.stringify(groups, null, 2), + "utf-8", + ); + + await Promise.all(Object.entries(emojiMetadata).map(([group, metadata]) => fs.writeFile( + `./data/v${version}/metadata/${group}.json`, + JSON.stringify(metadata, null, 2), + "utf-8", + ))); + } catch (error) { + throw new Error(`Failed to write metadata files: ${error.message}`); + }
215-267: LGTM! Version management implementation looks good.The version commands are well-implemented with proper error handling and consistent logging using consola.
271-277: LGTM! Common options implementation is clean and well-typed.The common options function is well-implemented with proper TypeScript types and clear documentation.
| import { readLockfile, writeLockfile } from "./utils/lockfile"; | ||
| import { readLockfile, writeLockfile } from "./lockfile"; | ||
| import { SHORTCODE_PROVIDERS_SCHEMA } from "./schemas"; | ||
| import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils"; |
There was a problem hiding this comment.
Fix missing export in utils.ts.
The import of getUnicodeVersionByEmojiVersion is causing build failures as it's not exported from the utils module.
Please ensure the function is exported in utils.ts:
// utils.ts
+ export function getUnicodeVersionByEmojiVersion(version: string): string | undefined {
// Implementation
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { getAllEmojiVersions, getUnicodeVersionByEmojiVersion } from "./utils"; | |
| // utils.ts | |
| // Other imports and existing code | |
| export function getUnicodeVersionByEmojiVersion(version: string): string | undefined { | |
| // Implementation | |
| // For example, you might map an emoji version to its corresponding unicode version | |
| // return someMapping[version]; | |
| } | |
| // Other exported functions and code |
🧰 Tools
🪛 GitHub Check: build
[failure] 14-14:
Module '"./utils"' has no exported member 'getUnicodeVersionByEmojiVersion'.
🪛 GitHub Actions: CI
[error] 14-14: Module './utils' has no exported member 'getUnicodeVersionByEmojiVersion'.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/versions.ts (1)
173-303:⚠️ Potential issuePrevent null reference in semver comparison.
When building the
versionsarray, some entries may haveemoji_versionset tonull(line 242). The final sort at line 301 attempts to compare\${b.emoji_version}.0`with`${a.emoji_version}.0``, potentially leading to an invalid semver string (e.g., "null.0").Here is one way to handle this:
return versions.sort((a, b) => { - return semver.compare(`${b.emoji_version}.0`, `${a.emoji_version}.0`); + const aVersion = a.emoji_version ? `${a.emoji_version}.0` : "0.0.0"; + const bVersion = b.emoji_version ? `${b.emoji_version}.0` : "0.0.0"; + return semver.compare(bVersion, aVersion); });
🧹 Nitpick comments (5)
test/__setup.ts (1)
1-6: LGTM! Consider adding JSDoc comments.The test setup correctly configures fetch mocking for the test environment. This is essential for testing network requests without making actual HTTP calls.
Consider adding JSDoc comments to explain the purpose of this setup file:
+/** + * Test setup file that configures fetch mocking. + * This allows tests to intercept and mock fetch requests + * without making actual HTTP calls. + */ import { vi } from "vitest"; import createFetchMock from "vitest-fetch-mock";vitest.config.ts (1)
1-9: LGTM! Consider adding JSDoc comments.The Vitest configuration correctly references the setup file and uses the recommended
defineConfighelper.Consider adding JSDoc comments to explain the configuration:
+/** + * Vitest configuration file. + * Configures the test environment by loading the setup file + * that enables fetch mocking for network requests. + */ import { defineConfig } from "vitest/config";src/versions.ts (1)
26-75: Refactor the repeated error handling.Within
getCurrentDraftVersion(), lines 43 & 50 both throw a similar error message ("failed to fetch draft readme or draft emoji readme"). Consider simplifying by consolidating them into a single check for readability and maintainability.if (rootResult == null || emojiResult == null) { throw new Error("failed to fetch draft readme or draft emoji readme"); } - if (rootResult.status === "rejected" || emojiResult.status === "rejected") { + if ( + rootResult.status !== "fulfilled" || + emojiResult.status !== "fulfilled" + ) { consola.error({ root: rootResult.status === "rejected" ? rootResult.reason : "ok", emoji: emojiResult.status === "rejected" ? emojiResult.reason : "ok", }); throw new Error("failed to fetch draft readme or draft emoji readme"); }test/versions.test.ts (2)
73-97: Expand test coverage to include corner cases.The current tests for
extractEmojiVersiondo not cover special emojis like"E0.7"or patch segments (e.g.,"E15.0.1"). Adding these cases helps ensure the regex logic remains robust.describe("extract emoji version", () => { it.each([ { input: "E14.0", expected: "14.0" }, { input: "E15.1", expected: "15.1" }, { input: "E5.0", expected: "5.0" }, + { input: "E0.7", expected: "0.7" }, + { input: "E15.0.1", expected: null }, ])("should extract valid emoji version numbers (input: $input, expected: $expected)", ({ input, expected }) => { expect(extractEmojiVersion(input)).toBe(expected); });
100-113: Consider removing or re-enabling commented-out tests.The block of code at lines 100–113 is commented out. If these tests are obsolete, consider deleting them to keep the test suite clean; otherwise, re-enable them to ensure coverage.
📜 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 (6)
package.json(2 hunks)src/utils.ts(1 hunks)src/versions.ts(1 hunks)test/__setup.ts(1 hunks)test/versions.test.ts(1 hunks)vitest.config.ts(1 hunks)
🔇 Additional comments (4)
package.json (2)
31-32: LGTM! Test scripts are well-configured.The test scripts follow common conventions and provide both single-run and watch mode options.
37-37:⚠️ Potential issue✅ Verification successful
Fix invalid version of consola package.
The specified version
3.4.0of theconsolapackage does not exist. The latest version is3.2.3.Apply this diff to fix the version:
- "consola": "^3.4.0", + "consola": "^3.2.3",
🌐 Web query:
What is the latest version of the consola package?💡 Result:
The latest version of the consola package is 3.4.0, as indicated by its
package.jsonfile in the GitHub repository[8] and listed on npmjs.com[2]. This version was published approximately one month prior to February 20, 2025[2][8].Key details:
- Installation: Available via npm/yarn/pnpm with
npm i consola[2][4]- Features: Includes browser support, interactive prompts, logging throttling, and multiple reporter options[2]
- Recent updates: Added smaller core builds for reduced bundle size and improved integration with testing frameworks like Jest/Vitest[2]
The version history shows continuous development since 2018[7], with v3.x introducing significant improvements like .NET 8 support in related tooling (though this appears in separate NUnit release notes)[1][3].
Citations:
- 1: https://docs.nunit.org/articles/nunit-engine/release-notes.html
- 2: https://www.npmjs.com/package/consola
- 3: https://github.com/nunit/nunit-console/releases
- 4: https://classic.yarnpkg.com/en/package/consola
- 5: https://digico.biz/base_product/all-console-upgrade-packages-last-release/
- 6: https://www.nuget.org/packages/xunit.runner.console
- 7: https://www.npmjs.com/package/consola/v/2.0.0
- 8: https://github.com/unjs/consola/blob/master/package.json
Consola version is valid.
The
package.jsonalready specifiesconsolaversion^3.4.0, which is the latest release according to the most recent information. No changes to the dependency are required.src/versions.ts (2)
77-99: Enhance regex to accommodate patch versions.Currently, the regex pattern
comment.match(/E(\d+\.\d)/);only handles two-segment versions likeE15.1. If you'd like to account for possible patch segments (for exampleE15.1.1), consider using a more flexible regex pattern.-const version = comment.match(/E(\d+\.\d)/); +const version = comment.match(/E(\d+\.\d+(?:\.\d+)?)/);
132-171: Align special case "0.7" with the rest of the code.
extractUnicodeVersionproperly maps"0.7" -> "7.0". Ensure upstream usage of"0.7"is recognized as a valid version during fetch or version checks to avoid unintended rejections when callingisEmojiVersionValidin other parts of the code.Would you like to run a quick script to confirm that all instances of
"0.7"usage align withextractUnicodeVersionlogic and don’t get prematurely invalidated?
| @@ -165,104 +54,9 @@ export async function isEmojiVersionValid(version: string): Promise<boolean> { | |||
| // from v1 to v5, there was only major releases. So no v1.1, v1.2, etc. | |||
| // only, v1.0, v2.0, v3.0, v4.0, v5.0. | |||
| // if version has any minor or patch, it is invalid. | |||
| if (semver.minor(version) !== 0 || semver.patch(version) !== 0) { | |||
| if (semver.major(version) <= 5 && (semver.minor(version) !== 0 || semver.patch(version) !== 0)) { | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent handling of special case "0.7".
isEmojiVersionValid() rejects any version ≤ 5 that has minor/patch components. However, extractUnicodeVersion() in src/versions.ts explicitly handles "0.7". This discrepancy may cause "0.7" to be invalid at one point and valid at another. Consider aligning these two behaviors for consistency.
if (semver.major(version) <= 5 && (semver.minor(version) !== 0 || semver.patch(version) !== 0)) {
- return false;
+ // Allow "0.7" to pass if it is an approved special case
+ if (!(semver.major(version) === 0 && semver.minor(version) === 7)) {
+ return false;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (semver.major(version) <= 5 && (semver.minor(version) !== 0 || semver.patch(version) !== 0)) { | |
| if (semver.major(version) <= 5 && (semver.minor(version) !== 0 || semver.patch(version) !== 0)) { | |
| // Allow "0.7" to pass if it is an approved special case | |
| if (!(semver.major(version) === 0 && semver.minor(version) === 7)) { | |
| return false; | |
| } | |
| } |
Summary by CodeRabbit