fix(types): derive defaultNS from generated resources when extract.defaultNS is false - #280
Merged
Merged
Conversation
Member
|
Merged, thanks — the analysis is right, and the reproduction made it easy to verify. I pushed two follow-ups on top in 4540650:
Two tests cover the new branches. Released in v1.67.9. |
Contributor
Author
|
Thanks for the quick merge, appreciate it! Found this one by running the CLI against one of my own projects, so glad the reproduction made it easy to verify. Best Regards, Tarik |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
When a project sets
extract.defaultNS: false(the namespace-less layout: one JSON file per language),i18next-cli typesemitsdefaultNS: falseinto the generatedCustomTypeOptions. i18next's type system cannot expressdefaultNS: false—DefaultNamespace = TypeOptions['defaultNS']feedsNs extends Namespace(string | readonly string[]) — so everyt()call silently accepts any string and the generated types provide zero key checking.This PR derives the emitted
defaultNSfrom the generatedResourceskeys instead (deduplicated, sorted, preferring the conventional'translation'), prints a warning naming the derived namespace, and keepsfalse(with a warning) only when the resource files are keyed per language (e.g.en.json+de.json), where deriving would turn locales into namespaces and produce false type errors. The derivation only runs wheni18next.d.tsis first created;resources.d.tscontent and--cibehavior are unchanged.Reproduction / verification
Found by running the CLI against a real project (
defaultNS: false, one file per language, ~220 keys):i18next-cli typesgenerateddefaultNS: false+Resources { "en": {...} }.t('this.key.does.not.exist')with those definitions (moduleResolution: bundler, i18next@26.3.6, TS 5.9.3 and 6.0.3) passed — bogus keys accepted everywhere.defaultNS: 'en'; the bogus key now fails with TS2345 while valid keys type-check. Multi-namespace shapes (defaultNS: ['hello','stacks']vs a single entry) were also compile-verified: unprefixed keys resolve against the first namespace only, prefixed sibling keys (t('stacks:title')) type-check, bogus keys are rejected.Checks run
npm test(eslint + vitest): 114 files / 1250 tests pass, including 5 new/updated tests intest/types-generator-ts.test.ts(single-file derivation, namespaced layout, per-language fallback, dedupe/sort, warn-only-on-create, empty resources).tsc --strictas above.Review
The diff was reviewed iteratively (4 rounds) by an automated reviewer (kiro-cli, claude-opus-5); its findings led to the dedupe/sort, the per-language guard, the warnings, and the extra tests. Final round reported no blocking issues.
Notes for reviewers
defaultNS: false;was emitted — that behavior was added in fbd7608 to fix the emission syntactically; the semantic gap (untypedt()) was not addressed there.'translation', the pick is alphabetically first — deterministic and called out by the warning, since the correct runtime default is not knowable statically.types.inputspanning multiple languages but withoutmergeNamespaces,Resourcesis keyed per language file; this PR intentionally does not change howresources.d.tsis built.