Skip to content

fix(plugin-i18n): change to builtinResources option #296

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

Merged
kazupon merged 1 commit into from
Aug 12, 2025
Merged

fix(plugin-i18n): change to builtinResources option #296

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
2 changes: 1 addition & 1 deletion packages/gunshi/src/cli.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ describe('auto generate usage', () => {
plugins: [
i18n({
locale: 'ja-JP',
resources: { 'ja-JP': jsJPResource }
builtinResources: { 'ja-JP': jsJPResource }
})
Comment on lines +558 to 559

@coderabbitai coderabbitai Bot Aug 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

LGTM: test config aligns with the new public option

Switching to builtinResources matches the plugin API rename and keeps tests up to date.

Run this repo-wide check to ensure there are no lingering usages of the old resources option in code, tests, or docs:

🏁 Script executed:

#!/bin/bash
# Find any remaining references to the old option name
rg -n --hidden --glob '!.git' --glob '!node_modules' --glob '!dist' $'\\bresources\\s*:' || true

# Confirm new option is used
rg -n --hidden --glob '!.git' --glob '!node_modules' --glob '!dist' $'\\bbuiltinResources\\s*:'

Length of output: 685

🏁 Script executed:

#!/bin/bash
# Inspect plugin-renderer source for "resources" usage
echo "=== plugin-renderer/src ==="
rg -n "resources" packages/plugin-renderer/src

# Show context in plugin-renderer README around the snippet
echo -e "\n=== plugin-renderer/README.md (lines 230-260) ==="
sed -n '230,260p' packages/plugin-renderer/README.md

# Show translation.ts around the private field
echo -e "\n=== plugin-i18n/src/translation.ts (lines 1-40) ==="
sed -n '1,40p' packages/plugin-i18n/src/translation.ts

Length of output: 2426

Documentation update required: replace deprecated resources option in plugin-renderer README

The test changes correctly switch to the new builtinResources API, but the docs still reference the old resources option. Please update:

• packages/plugin-renderer/README.md (around line 243):

-    resources: customResources
+    builtinResources: customResources

No changes are needed for the private #resources field in plugin-i18n/src/translation.ts or the lockfile entry.

📝 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.

Suggested change
builtinResources: { 'ja-JP': jsJPResource }
})
--- packages/plugin-renderer/README.md
@@ -243,1 +243,1
- resources: customResources
+ builtinResources: customResources
🤖 Prompt for AI Agents 
In packages/plugin-renderer/README.md around line 243, the documentation still
references the deprecated resources option; update all examples and option docs
to use builtinResources instead (e.g., replace "resources: { 'ja-JP': ... }"
with "builtinResources: { 'ja-JP': ... }"), search the README for any other
occurrences of "resources" related to this plugin-renderer API and replace them,
and ensure the example code blocks and descriptive text reflect the new API name
and usage consistently.

]
})
Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-i18n/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ await cli(process.argv.slice(2), greetCommand, {
- Default: `'en-US'`
- Description: The locale to use for translations. Can be a BCP 47 language tag string or an `Intl.Locale` object.

### `resources`
### `builtinResources`

- Type: `Record<string, Record<BuiltinResourceKeys, string>>`
- Default: `{}`
Expand Down Expand Up @@ -120,7 +120,7 @@ await cli(args, command, {
globals(), // Adds --help and --version options
i18n({
locale: 'ja-JP',
resources: {
builtinResources: {
'ja-JP': jsJPResource // Set from with providing gunshi built-in resources
}
})
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-i18n/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('extension: translate', () => {
const jaJPResource = await import('@gunshi/resources/ja-JP', { with: { type: 'json' } }).then(
m => m.default || m
)
const plugin = i18n({ locale: 'ja-JP', resources: { 'ja-JP': jaJPResource } })
const plugin = i18n({ locale: 'ja-JP', builtinResources: { 'ja-JP': jaJPResource } })
const ctx = await createMockCommandContext()
const extension = await plugin.extension.factory(ctx, {} as Command)

Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-i18n/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ export default function i18n(
const locale = toLocale(options.locale)
const localeStr = toLocaleString(locale)

const resources =
options.resources ||
const builtinResources =
options.builtinResources ||
(Object.create(null) as Record<string, Record<BuiltinResourceKeys, string>>)
Comment on lines +86 to 88

@coderabbitai coderabbitai Bot Aug 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Provide a backward-compatible fallback with a deprecation warning

To avoid breaking existing users immediately, fall back to the deprecated resources option when builtinResources is not provided, and warn once. This pairs with the type alias change.

Apply this diff:

-  const builtinResources =
-    options.builtinResources ||
-    (Object.create(null) as Record<string, Record<BuiltinResourceKeys, string>>)
+  const builtinResources =
+    options.builtinResources ??
+    // Deprecated alias (to be removed in the next major)
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    (options as any).resources ??
+    (Object.create(null) as Record<string, Record<BuiltinResourceKeys, string>>)
+
+  // Soft deprecation notice (printed once)
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  if ((options as any).resources && !options.builtinResources) {
+    // Keep this lightweight to avoid noisy logs; consider gating by env if needed
+    console.warn(
+      '[@gunshi/plugin-i18n] `resources` option is deprecated. Use `builtinResources` instead.'
+    )
+  }
📝 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.

Suggested change
const builtinResources =
options.builtinResources ||
(Object.create(null) as Record<string, Record<BuiltinResourceKeys, string>>)
const builtinResources =
options.builtinResources ??
// Deprecated alias (to be removed in the next major)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(options as any).resources ??
(Object.create(null) as Record<string, Record<BuiltinResourceKeys, string>>)
// Soft deprecation notice (printed once)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
if ((options as any).resources && !options.builtinResources) {
// Keep this lightweight to avoid noisy logs; consider gating by env if needed
console.warn(
'[@gunshi/plugin-i18n] `resources` option is deprecated. Use `builtinResources` instead.'
)
}
🤖 Prompt for AI Agents 
In packages/plugin-i18n/src/index.ts around lines 86 to 88, the code currently
defaults builtinResources to an empty object which breaks backward
compatibility; update it to fall back to the deprecated resources option when
builtinResources is undefined, and emit a single deprecation warning the first
time this fallback is used. Implement a one-time warning (e.g., using a
module-scoped boolean flag) so the log only appears once, set builtinResources =
options.builtinResources ?? options.resources ?? Object.create(null), and call
the warning when options.builtinResources is undefined but options.resources is
provided.


// create translation adapter
Expand Down Expand Up @@ -150,7 +150,7 @@ export default function i18n(
setResource(DEFAULT_LOCALE, DefaultResource as Record<BuiltinResourceKeys, string>)

// install built-in locale resources
for (const [locale, resource] of Object.entries(resources)) {
for (const [locale, resource] of Object.entries(builtinResources)) {
setResource(locale, resource)
}

Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-i18n/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export interface I18nPluginOptions {
/**
* Built-in localizable resources
*/
resources?: Record<string, Record<BuiltinResourceKeys, string>>
builtinResources?: Record<string, Record<BuiltinResourceKeys, string>>
}
Comment on lines +93 to 94

@coderabbitai coderabbitai Bot Aug 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Public option rename looks good; add a deprecated alias to avoid a breaking change

Renaming a public option is a breaking change for downstream consumers. Consider keeping a deprecated resources? alias in the type to provide a migration window while promoting builtinResources. This preserves strict typing and eases adoption.

Apply this diff to introduce a deprecated alias:

 export interface I18nPluginOptions {
   /**
    * Locale to use for translations
    */
   locale?: string | Intl.Locale
   /**
    * Translation adapter factory
    */
   translationAdapterFactory?: TranslationAdapterFactory
   /**
    * Built-in localizable resources
    */
-  builtinResources?: Record<string, Record<BuiltinResourceKeys, string>>
+  /**
+   * @deprecated Use `builtinResources` instead. This alias will be removed in the next major release.
+   */
+  resources?: Record<string, Record<BuiltinResourceKeys, string>>
+  builtinResources?: Record<string, Record<BuiltinResourceKeys, string>>
 }
📝 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.

Suggested change
builtinResources?: Record<string, Record<BuiltinResourceKeys, string>>
}
export interface I18nPluginOptions {
/**
* Locale to use for translations
*/
locale?: string | Intl.Locale
/**
* Translation adapter factory
*/
translationAdapterFactory?: TranslationAdapterFactory
/**
* Built-in localizable resources
*/
/**
* @deprecated Use `builtinResources` instead. This alias will be removed in the next major release.
*/
resources?: Record<string, Record<BuiltinResourceKeys, string>>
builtinResources?: Record<string, Record<BuiltinResourceKeys, string>>
}
🤖 Prompt for AI Agents 
In packages/plugin-i18n/src/types.ts around lines 93-94, the public option was
renamed to builtinResources which is a breaking change for consumers; add an
optional deprecated alias resources with the identical type signature
(Record<string, Record<BuiltinResourceKeys, string>>) so existing callers keep
working; annotate the alias with a JSDoc @deprecated comment pointing users to
use builtinResources and keep both properties optional to preserve strict typing
and provide a migration window.


/**
Expand Down