Skip to content

Convert punctuateSeries to TypeScript #4351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vtejasri1511-sudo wants to merge 1 commit into
base: 2.x
Choose a base branch
from
+34 −37
Open

Convert punctuateSeries to TypeScript #4351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 0 additions & 37 deletions framework/core/js/src/common/helpers/punctuateSeries.js
View file
Open in desktop

This file was deleted.

34 changes: 34 additions & 0 deletions framework/core/js/src/common/helpers/punctuateSeries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import type { Children } from 'mithril';
import app from '../../common/app';

/**
* Formats a list of strings/nodes to read fluently in the locale.
*/
export default function punctuateSeries(items: Children[] = []): Children {
Copy link
Member

@imorland imorland Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function-level JSDoc describing the purpose and example has been removed. Since dist-typings is generated from this file, that documentation will be lost for extension authors browsing the .d.ts declarations. Restore the description and @example from the original .js file:

/**
 * The `punctuateSeries` helper formats a list of strings (e.g. names) to read
 * fluently in the application's locale.
 *
 * ```js
 * punctuateSeries(['Toby', 'Franz', 'Dominion']) // Toby, Franz, and Dominion
 * ```
 */

Copy link
Member

@imorland imorland Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The items parameter has a default of []. The original function required an argument. While defensive, this is a subtle API change — callers passing undefined will now silently get [] back. Since all current call sites pass the array explicitly, consider dropping the default to keep the types tight:

export default function punctuateSeries(items: Children[]): Children {

Copy link
Member

@imorland imorland Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function-level JSDoc describing the purpose and example has been removed. Since dist-typings is generated from this file, that documentation will be lost for extension authors browsing the .d.ts declarations. Restore the description and @example from the original .js file:

/**
 * The `punctuateSeries` helper formats a list of strings (e.g. names) to read
 * fluently in the application's locale.
 *
 * ```js
 * punctuateSeries(['Toby', 'Franz', 'Dominion']) // Toby, Franz, and Dominion
 * ```
 */

if (items.length === 2) {
return app.translator.trans('core.lib.series.two_text', {
first: items[0],
second: items[1],
});
}

if (items.length >= 3) {
const glue = app.translator.trans('core.lib.series.glue_text') as Children;

Copy link
Member

@imorland imorland Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trans() with parameters returns any[] (see Translator.tsx overloads), not Children. The as Children assertion is masking an underlying type mismatch rather than resolving it.

The simplest correct approach is to type glue as any[]:

const glue = app.translator.trans('core.lib.series.glue_text') as any[];

Or, since the translator returns the rendered translation here, you can just use any and let the array push accept it — or keep as Children with a comment explaining why. Either way, a note clarifying the intentional cast would help reviewers.

Copy link
Member

@imorland imorland Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trans() with parameters is typed as any[] (see Translator.tsx overloads), not Children. The cast as Children is masking an underlying type mismatch rather than resolving it.

The simplest correct approach is to type glue as any[]:

const glue = app.translator.trans('core.lib.series.glue_text') as any[];

Or keep as Children but add a comment explaining the intentional cast so reviewers understand it's deliberate.

// Build middle items safely: [item, glue, item, glue, ...]
const second: Children[] = [];
for (let i = 1; i < items.length - 1; i++) {
second.push(items[i]);
if (i < items.length - 2) second.push(glue);
}

return app.translator.trans('core.lib.series.three_text', {
first: items[0],
second,
third: items[items.length - 1],
});
}

// 0 or 1 item: return as-is (Mithril can render arrays too)
return items;
}
Loading