diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7d7b3f4..1286c4c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,10 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - node-version: [18] + # 18 is the documented minimum; 24 is the current LTS and the only leg + # where `require()` of an ESM dependency works, so it is what actually + # exercises the published CJS entry. + node-version: [18, 24] os: [ubuntu-latest, windows-latest, macOS-latest] steps: @@ -33,3 +36,8 @@ jobs: run: npm run test env: tests: true + + - name: Test published bundle + run: npm run test:dist + env: + tests: true diff --git a/AGENTS.md b/AGENTS.md index 59022b8..ba83fc2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,7 +34,7 @@ translation state, loading, caching, route matching, and preprocessing — but | Tests | Jest + `ts-jest` ESM preset, `testEnvironment: node` | | Lint | ESLint `airbnb-typescript/base` | | Runtime peer | `svelte >=3.49.0` (uses `svelte/store` only) | -| CI | `.github/workflows/tests.yml` — Node 18, ubuntu/macOS/windows | +| CI | `.github/workflows/tests.yml` — Node 18 + 24, ubuntu/macOS/windows | ## Commands @@ -42,7 +42,9 @@ translation state, loading, caching, route matching, and preprocessing — but |---------|---------| | `npm install` / `npm ci` | install (respects `package-lock.json`) | | `npm run build` | tsup build to `dist/` | -| `npm test` | jest suite | +| `npm test` | jest suite (runs `typecheck` first) | +| `npm run test:dist` | bundle-level suite (builds first) — not part of `npm test` | +| `npm run typecheck` | `tsc --noEmit` over `src` **and** the repo's `*.js` configs | | `npm run lint` | eslint `--fix` over `.ts`/`.js` (also a pre-commit hook) | ## Repository map @@ -54,9 +56,11 @@ translation state, loading, caching, route matching, and preprocessing — but | `src/logger.ts` | `loggerFactory` + module-level `logger` singleton + `setLogger` | | `src/types.ts` | all public/internal types | | `tests/specs/index.spec.ts` | the suite (one `describe`) | +| `tests/specs/dist.spec.ts` | bundle-level checks — runs only via `npm run test:dist` | | `tests/data/` | `CONFIG` + JSON fixtures + `getTranslations()` | | `docs/README.md` | public API reference — keep in sync with code | | `dist/` | generated build output — never hand-edit | +| `tsconfig.tools.json` | type-checks the repo's own `*.js` config files | ## Architecture you must respect @@ -228,6 +232,14 @@ not RCE/XSS. user-controlled key must use an own-property check (`Object.prototype. hasOwnProperty.call`, or the `hasOwn` helper) so `toString`/`__proto__`/ `constructor` are treated as missing, not inherited members. +- **Never bracket-assign a user-supplied locale or key onto a plain object.** + `table[locale] = value` routes `'__proto__'` through the prototype setter, so + no own key is recorded and the object's prototype is replaced. Write with a + computed-key spread (`{ ...table, [locale]: value }`, which is + `DefineProperty`) or target an `Object.create(null)` object — that is why + `loadedKeys` has a null prototype. The locale-indexed accumulators in + `serialize` and `getTranslationProps` are plain objects and stay correct only + while they follow this rule. - **Fail soft at the edges.** A single throwing loader must not wipe a whole batch; a missing config/parser must not throw on `t.get()`/`l.get()`. - **`route` reaches `RegExp.test()` and is visitor-controlled** (`url.pathname`) @@ -246,7 +258,11 @@ not RCE/XSS. ## 13. Tests -- Tests live in `tests/specs/index.spec.ts`; fixtures in `tests/data/`. +- Tests live in `tests/specs/index.spec.ts`; fixtures in `tests/data/`. The one + exception is `tests/specs/dist.spec.ts`, which asserts on the built bundle + (toolchain-level guarantees the sources cannot prove) and therefore runs + separately via `npm run test:dist`, which builds first. Everything else + belongs in `index.spec.ts`. - Drive behavior through the **public API** (`new i18n(CONFIG)`, stores, methods). Pure helpers may be imported directly from `src/` when that yields a more deterministic test (e.g. unit-testing `loggerFactory`). @@ -256,7 +272,12 @@ not RCE/XSS. **alongside** the fix — never a fix without it. Escape hatch (SSR hydration, infra, purely visual): say so explicitly in the PR with manual repro steps. - Don't assert on the shared module-level `logger` singleton — it leaks across - async tests; construct a logger/instance locally instead. + async tests; construct a logger/instance locally instead. When the output + under test only reaches that singleton, install it through the spec's + `captureLogs` helper (which restores the previous logger, not a fresh one) + and assert on messages matched by content. Counting is fine once filtered to + the message under test; never assert on a raw call count, which any + still-running load from an earlier test can change. - `t`/`l` resolve via a no-op test parser (`parse: (...) => key`) — design assertions accordingly. diff --git a/docs/README.md b/docs/README.md index e665e55..9360339 100644 --- a/docs/README.md +++ b/docs/README.md @@ -210,6 +210,19 @@ This will match: - `/shop` - `/shop/cart` +**⚠️ Security note:** Route patterns are tested against the current route — typically the visitor-controlled `url.pathname`. A regular expression with catastrophic backtracking (e.g. nested quantifiers like `/^\/(a+)+$/` — a run of `a`s followed by a non-matching character stalls it for seconds) lets a crafted URL stall your server (ReDoS). Keep route regexes simple and anchored, and avoid nested quantifiers. + +Route patterns are matched statelessly and are never written to: a pattern +carrying the `g` or `y` flag matches the same way on every navigation instead of +alternating, its `lastIndex` is left untouched for your own use, and a frozen +pattern works like any other. Flag semantics are preserved — `y` still anchors +the match at the start of the route. + +An object that is not a regular expression is asked for a `test(route)` method +like any other, so a custom matcher works; one that cannot answer is reported as +an invalid route config. Entries that are neither strings nor objects simply +never match. + **No routes (global):** ```javascript @@ -852,7 +865,11 @@ Array of all available locales (from loaders). ### `loading` -**Type:** `Readable & { toPromise: () => Promise, get: () => boolean }` +**Type:** `Readable & { toPromise: (locale?: string, route?: string) => Promise, get: () => boolean }` + +With overlapping loads the flag belongs to whichever load last touched it, so +prefer awaiting the promise returned by `loadTranslations()` for the load you +actually care about. Loading state indicator. @@ -964,9 +981,11 @@ All loaded translations before preprocessing. ### `loadTranslations` -**Type:** `(locale: string, route?: string) => Promise` +**Type:** `(locale: string, route?: string) => Promise | undefined` -Load translations for a specific locale and route. +Load translations for a specific locale and route. Returns `undefined` when the +locale cannot be resolved (unknown and no matching `fallbackLocale`), so use +`await` rather than chaining onto the result directly. **Usage in layouts:** @@ -991,13 +1010,33 @@ export const load = async ({ url }) => { 4. Preprocesses translations 5. Updates stores +**Errors:** a loader that throws is caught and logged individually, so one +broken loader does not fail the batch. Anything that throws afterwards — a +custom `preprocess`, a malformed payload — **rejects the returned promise**, so +`await loadTranslations(...)` surfaces it (in SvelteKit, straight to the error +boundary). A result you discard is safe — the failure is logged through the +configured logger and never surfaces as an unhandled rejection — but it is then +only visible in the log. + +A subscriber of your own that throws is the exception, and where the error +surfaces depends on which store it subscribes to. `loading` is written while the +load is still starting up, inside the store write your call triggered, so a +throwing `loading` subscriber propagates **synchronously** out of +`loadTranslations()` / `setLocale()` / `setRoute()` — there is no promise to +reject yet. `translations` and `locale` are written after the fetch resolves, so +a throwing subscriber there **rejects the returned promise** like any other +late failure. Either way the throw leaves svelte's subscriber queue mid-flush, +which stalls later store updates, so keep subscribers total. + --- ### `setLocale` -**Type:** `(locale: string) => Promise` +**Type:** `(locale?: string) => Promise | undefined` -Set locale and load its translations. +Set locale and load its translations. Returns `undefined` when no locale is +given or the locale is already active; errors behave as in +[`loadTranslations`](#loadtranslations). **Usage:** @@ -1017,9 +1056,11 @@ await setLocale('cs'); ### `setRoute` -**Type:** `(route: string) => Promise` +**Type:** `(route: string) => Promise | undefined` -Update current route and load route-specific translations. +Update current route and load route-specific translations. Returns `undefined` +when the route is already active; errors behave as in +[`loadTranslations`](#loadtranslations). **Usage:** diff --git a/jest.config.js b/jest.config.js index 746b6e5..4b6a9b9 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,5 +1,14 @@ +import { createDefaultEsmPreset } from 'ts-jest'; + /** @type {import('ts-jest').JestConfigWithTsJest} */ export default ({ - preset: 'ts-jest/presets/default-esm', + // Compiled with `tests/tsconfig.json` (see the comment there). The preset + // helper owns the transform pattern, so an upgrade cannot leave a second, + // stale entry behind that the `tsconfig` override would not reach. + ...createDefaultEsmPreset({ tsconfig: '/tests/tsconfig.json' }), testEnvironment: 'node', -}); \ No newline at end of file + // `dist.spec.ts` asserts on a built bundle and only runs through + // `npm run test:dist`, which builds it first; a bare `jest` would otherwise + // resolve nothing on a fresh clone or assert against a stale artifact. + testPathIgnorePatterns: ['/node_modules/', '/tests/specs/dist\\.spec\\.ts$'], +}); diff --git a/package.json b/package.json index 5b14233..85bd83b 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,11 @@ }, "scripts": { "dev": "tsup --watch", + "typecheck": "tsc --noEmit -p tsconfig.json && tsc --noEmit -p tsconfig.tools.json", + "pretest": "npm run typecheck", "test": "npx cross-env NODE_OPTIONS=--experimental-vm-modules jest", + "pretest:dist": "npm run build", + "test:dist": "npx cross-env NODE_OPTIONS=--experimental-vm-modules jest tests/specs/dist.spec.ts --testPathIgnorePatterns=/node_modules/", "build": "tsup", "prepublishOnly": "npm run build", "lint": "eslint --fix --ext .ts,.js --ignore-path .gitignore ." diff --git a/src/index.ts b/src/index.ts index 346397e..9959add 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,6 @@ import { derived, get, writable } from 'svelte/store'; import { checkProps, fetchTranslations, hasOwn, read, sanitizeLocales, testRoute, toDotNotation, translate } from './utils'; -import { logger, loggerFactory, setLogger } from './logger'; +import { logError, logger, loggerFactory, setLogger } from './logger'; import type { Config, Loader, Parser, Translations, LoadingStore, ExtendedStore, Logger } from './types'; import type { Readable, Writable } from 'svelte/store'; @@ -14,21 +14,33 @@ export default class I18n { this.loaderTrigger.subscribe(this.loader); // purge resolved promises - this.isLoading.subscribe(async ($loading) => { - if ($loading && this.promises.size) { - await this.loading.toPromise(); - this.promises.clear(); + this.isLoading.subscribe(($loading) => { + if (!$loading || !this.promises.size) return; + + const tracked = Array.from(this.promises); + + // Every promise has to SETTLE before the set is cleared — `Promise.all` + // rejects on the first failure, which would drop loads still in flight + // and make a later `toPromise()` resolve before their data arrived. + // A plain `.then` chain keeps this out of a discarded async subscriber. + Promise.all(tracked.map(({ promise }) => promise.catch(() => {}))).then(() => { + tracked.forEach((entry) => this.promises.delete(entry)); logger.debug('Loader promises have been purged.'); - } + }); }); + // `loadConfig` reports and marks its own failure, so a config load started + // here — with no caller to reject to — is covered by the same path as a + // consumer's fire-and-forget call. if (config) this.loadConfig(config); } private cachedAt = 0; - private loadedKeys: Loader.IndexedKeys = {}; + // Null prototype: this cache is indexed by user-supplied locales, and a + // plain object would resolve a '__proto__' assignment via the setter. + private loadedKeys: Loader.IndexedKeys = Object.create(null); private currentRoute: Writable = writable(); @@ -41,7 +53,9 @@ export default class I18n { loading: LoadingStore = { subscribe: this.isLoading.subscribe, toPromise: (locale, route) => { - const { fallbackLocale } = get(this.config); + // A route or locale can be set before the config resolves, so this must + // not assume one exists. + const { fallbackLocale } = get(this.config) || {}; const promises = Array.from(this.promises).filter( (promise) => { @@ -52,7 +66,15 @@ export default class I18n { }, ).map(({ promise }) => promise); - return Promise.all(promises); + const output = Promise.all(promises); + + // `setLocale`/`setRoute`/`loadTranslations` return this and are routinely + // called fire-and-forget. The underlying failure is already reported by + // `loader`, so mark the aggregate handled; anyone awaiting it still gets + // the rejection. + output.catch(() => {}); + + return output; }, get: () => get(this.isLoading), }; @@ -208,8 +230,17 @@ export default class I18n { if (initLocale) await this.loadTranslations(initLocale); } - loadConfig = async (config: Config.T) => { - await this.configLoader(config); + loadConfig = (config: Config.T) => { + const promise = this.configLoader(config); + + // Documented as a public method and routinely called fire-and-forget, so + // the failure is reported and the promise marked handled here; anyone + // awaiting it still receives the rejection. A synchronous failure in + // `configLoader` never reaches the loader's own handler, so without this it + // would leave a silently half-initialized instance. + promise.catch((error) => logError('Failed to load the i18n config.', error)); + + return promise; }; getTranslationProps = async ($locale = this.locale.get(), $route = get(this.currentRoute)): Promise<[Translations.SerializedTranslations, Loader.IndexedKeys] | []> => { @@ -228,7 +259,7 @@ export default class I18n { this.cachedAt = Date.now(); } else if (Date.now() > cacheValue + this.cachedAt) { logger.debug('Refreshing cache.'); - this.loadedKeys = {}; + this.loadedKeys = Object.create(null); this.cachedAt = 0; } @@ -258,13 +289,13 @@ export default class I18n { try { rawTranslations = await fetchTranslations(filteredLoaders); } finally { - // Always release the loading flag, even if fetching throws, so the - // instance never gets stuck in a permanent loading state. + // Always release the flag, even if fetching throws, so the instance + // never gets stuck in a permanent loading state. this.isLoading.set(false); } - const loadedKeys = Object.keys(rawTranslations).reduce( - (acc, locale) => ({ ...acc, [locale]: Object.keys(rawTranslations[locale]) }), {} as Loader.IndexedKeys, + const loadedKeys = Object.entries(rawTranslations).reduce( + (acc, [locale, data]) => ({ ...acc, [locale]: Object.keys(data) }), {} as Loader.IndexedKeys, ); const keys = filteredLoaders @@ -273,7 +304,7 @@ export default class I18n { )) .reduce>((acc, { key, locale }) => ({ ...acc, - [locale]: [...(acc[locale] || []), key], + [locale]: [...(read(acc, locale) || []), key], }), {}); return [rawTranslations, keys]; @@ -296,8 +327,8 @@ export default class I18n { (acc, locale) => ({ ...acc, [locale]: { - ...(acc[locale] || {}), - ...translations[locale], + ...(read(acc, locale) || {}), + ...read(translations, locale), }, }), $rawTranslations, @@ -306,7 +337,7 @@ export default class I18n { this.privateTranslations.update(($translations) => translationLocales.reduce( (acc, locale) => { let dotnotate = true; - let input = translations[locale]; + let input = read(translations, locale); if (typeof preprocess === 'function') { input = preprocess(input); @@ -319,7 +350,7 @@ export default class I18n { return ({ ...acc, [locale]: { - ...(acc[locale] || {}), + ...(read(acc, locale) || {}), ...dotnotate ? toDotNotation(input, preprocess === 'preserveArrays') : input, }, }); @@ -328,7 +359,9 @@ export default class I18n { )); translationLocales.forEach(($locale) => { - let localeKeys: Loader.Key[] | undefined = Object.keys(translations[$locale]).map((k) => `${k}`.split('.')[0]); + // A `null` payload for a locale must not take the whole call down: every + // step above tolerates it, so this bookkeeping read does too. + let localeKeys: Loader.Key[] | undefined = Object.keys(read(translations, $locale) || {}).map((k) => `${k}`.split('.')[0]); if (keys) localeKeys = read(keys, $locale); this.loadedKeys[$locale] = Array.from(new Set([ @@ -338,29 +371,65 @@ export default class I18n { }); }; - private loader = async ([inputLocale, route]: string[]) => { - const locale = this.getLocale(inputLocale) || undefined; - - logger.debug(`Adding loader promise for '${locale}' locale and '${route}' route.`); + // Not `async`: `subscribe` discards whatever the subscriber returns, so + // anything thrown outside `promise` — `getLocale`, a custom logger — would + // reject a promise nobody can reach. + private loader = ([inputLocale, route]: string[]) => { + let locale: Config.Locale | undefined; const promise = (async () => { + try { + locale = this.getLocale(inputLocale) || undefined; + } catch (error) { + // Runs before the first `await`, so this still lands before the entry + // is recorded below. `loading.toPromise` matches on the sanitized + // locale, so filing the failure under the raw input would filter the + // rejection out and report success to the caller. + [locale] = sanitizeLocales(inputLocale); + + throw error; + } + + logger.debug(`Adding loader promise for '${locale}' locale and '${route}' route.`); + const props = await this.getTranslationProps(locale, route); + if (props.length) this.addTranslations(...props); + + if (locale && this.locale.get() !== locale) this.locale.forceSet(locale); })(); + // Marks the promise handled so a load nobody awaits cannot terminate the + // process, without swallowing it: callers awaiting `loadTranslations` or + // `loading.toPromise()` still receive the rejection. + promise.catch((error) => logError(`Failed to load translations for '${locale}' locale and '${route}' route.`, error)); + + // `locale` is assigned before the first `await` above, so it is already + // resolved here and the entry stays filterable by `loading.toPromise`. this.promises.add({ locale, route, promise, }); - - promise.then(() => { - if (locale && this.locale.get() !== locale) this.locale.forceSet(locale); - }); }; loadTranslations = (locale: Config.Locale, route = get(this.currentRoute) || '') => { - const normalizedLocale = this.getLocale(locale); + let normalizedLocale: Config.Locale | undefined; + + try { + normalizedLocale = this.getLocale(locale); + } catch (error) { + // Resolving the locale reads consumer config, so it can throw. This is + // the primary public entry point: the failure has to arrive the same way + // a failed load does, as a reported rejection rather than a synchronous + // throw the caller cannot attach a handler to. + logError(`Failed to load translations for '${locale}' locale and '${route}' route.`, error); + + const failed = Promise.reject(error); + failed.catch(() => {}); + + return failed; + } if (!normalizedLocale) return; diff --git a/src/logger.ts b/src/logger.ts index 3fba49f..26d4d9c 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -10,9 +10,19 @@ export const loggerFactory = ({ logger = console, level = loggerLevels[1], prefi ...acc, [key]: (value: any) => { if (levelIndex < i) return; - // Custom loggers may not implement every level; skip rather than throw. - if (typeof logger[key] !== 'function') return; - return logger[key](`${prefix}${value}`); + try { + // Inside the `try`: reading the method is consumer-controlled too — the + // logger itself can be null, or expose the level through an accessor. + // Custom loggers may not implement every level; skip rather than throw. + if (typeof logger[key] !== 'function') return undefined; + + return logger[key](`${prefix}${value}`); + } catch (error) { + // A consumer's logger must never be able to take the library down. + // Several call sites log from promise handlers that nothing awaits, + // where a throw would surface as an unhandled rejection. + return undefined; + } }, }), {} as Logger.T); }; @@ -20,3 +30,15 @@ export const loggerFactory = ({ logger = console, level = loggerLevels[1], prefi export let logger = loggerFactory({}); export const setLogger = (l: Logger.T) => { logger = l; }; + +// Single shape for every reported failure: context first, then the error, which +// the configured logger formats like any other value. Cannot throw — +// `loggerFactory` contains a custom logger's failures. +// +// Deliberately not deduplicated: a shared error object (a module-level +// constant, a cached rejection) would then be reported once for the whole +// process, and a failure nobody awaits has no other signal. +export const logError = (message: string, error?: any) => { + logger.error(message); + logger.error(error); +}; diff --git a/src/utils.ts b/src/utils.ts index 36afc86..0e2f246 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,5 +1,5 @@ import type { DotNotation, Translations, Loader } from './types'; -import { logger } from './logger'; +import { logError, logger } from './logger'; // Safe own-property read. Translation keys like `toString`, `constructor` or // `__proto__` would otherwise resolve to inherited `Object.prototype` members @@ -124,8 +124,7 @@ export const fetchTranslations: Translations.FetchTranslations = async (loaders) try { data = await loader(); } catch (error) { - logger.error(`Failed to load translation. Verify your '${rest.locale}' > '${rest.key}' Loader.`); - logger.error(error); + logError(`Failed to load translation. Verify your '${rest.locale}' > '${rest.key}' Loader.`, error); } return { loader, ...rest, data }; })); @@ -136,7 +135,24 @@ export const fetchTranslations: Translations.FetchTranslations = async (loaders) export const testRoute = (route: string) => (input: Loader.Route) => { try { if (typeof input === 'string') return input === route; - if (typeof input === 'object') return input.test(route); + if (typeof input === 'object') { + // `test` advances `lastIndex` on a `g`/`y` pattern, so a route object + // shared across navigations would match only every other time and the + // consumer's own use of it would be corrupted. A throwaway copy starts at + // `lastIndex === 0` every time, keeps the flags' meaning (sticky still + // anchors), and never writes to the original — which also matters when + // the original is frozen. + // + // Only an actual RegExp is copied: `source`/`flags` on anything else are + // not a pattern, and `new RegExp(undefined)` is `/(?:)/`, which matches + // every route. Everything else is asked for `test` as-is, with the route + // passed through unchanged — a custom matcher may distinguish an unset + // route from the string 'undefined'. + const stateful = input instanceof RegExp && (input.global || input.sticky); + const pattern = stateful ? new RegExp(input.source, input.flags) : input; + + return pattern.test(route); + } } catch (error) { logger.error('Invalid route config!'); } diff --git a/tests/specs/dist.spec.ts b/tests/specs/dist.spec.ts new file mode 100644 index 0000000..96a75ab --- /dev/null +++ b/tests/specs/dist.spec.ts @@ -0,0 +1,107 @@ +import { execFileSync } from 'child_process'; +import { fileURLToPath } from 'url'; +// eslint-disable-next-line import/extensions -- the built ESM bundle is the subject under test +import I18n from '../../dist/index.js'; + +// The suite compiles `src` with a newer target than the published bundle, so +// prototype safety proven there says nothing about what ships. esbuild lowers +// object spread to its own helper; these assertions pin that the built artifact +// keeps a prototype-named locale as an own property. +describe('published bundle', () => { + const parser = { parse: (text: any, _params: any, _locale: any, key: string) => (text === undefined ? key : text) }; + const log = { level: 'error' as const }; + + it('keeps a locale named like an `Object.prototype` member as an own key', async () => { + const instance = new (I18n as any)({ + parser, + log, + loaders: [{ key: 'common', locale: '__proto__', loader: async () => ({ greeting: 'Hi' }) }], + }); + + await instance.loadTranslations('__proto__'); + + const table = instance.translations.get(); + const descriptor = Object.getOwnPropertyDescriptor(table, '__proto__'); + + expect(descriptor).toBeDefined(); + expect(descriptor?.value).toEqual( + expect.objectContaining({ 'common.greeting': 'Hi' }), + ); + expect(Object.getPrototypeOf(table)).toBe(Object.prototype); // not reparented + }); + + it('keeps a literal `__proto__` translation key as an own property', async () => { + const instance = new (I18n as any)({ parser, log }); + + instance.addTranslations({ en: JSON.parse('{"__proto__": "boom", "plain": "ok"}') }); + + const table = instance.translations.get().en; + + expect(Object.getOwnPropertyDescriptor(table, '__proto__')?.value).toBe('boom'); + expect(table.plain).toBe('ok'); + expect(Object.getPrototypeOf(table)).toBe(Object.prototype); // not reparented + }); +}); + +// `dist/index.cjs` is a separate esbuild pass with its own copy of the spread +// helper, so the ESM assertions above say nothing about it. It has to run in a +// real Node process: jest's ESM runtime cannot `require()` the bundle, and +// neither can a Node old enough to reject `require()` of an ESM dependency +// (svelte ships its store as ESM only) — where that is the case there is no +// loadable CJS entry to assert on. +const probeCjs = () => { + // Resolved relative to this spec, like the ESM import above — `process.cwd()` + // is wherever jest was invoked from, which is not necessarily the package + // root. JSON-encoded because the path contains backslashes on Windows, which + // a raw interpolation would turn into escape sequences (`\\a` -> `a`). + const entry = JSON.stringify(fileURLToPath(new URL('../../dist/index.cjs', import.meta.url))); + + // The child reports which outcome happened rather than the spec guessing from + // an error message, and the reason is checked: ONLY this Node being unable to + // `require()` svelte's ESM-only store is tolerated. A missing, malformed or + // otherwise broken bundle has to fail, or it would ship as a green skip. + const script = ` + let bundle; + try { + bundle = require(${entry}); + } catch (error) { + console.log(JSON.stringify({ loadable: false, reason: String((error && error.code) || error) })); + process.exit(0); + } + + const I18n = bundle.default || bundle; + const instance = new I18n({ parser: { parse: (text, params, locale, key) => (text === undefined ? key : text) }, log: { level: 'error' } }); + instance.addTranslations({ en: JSON.parse('{"__proto__": "boom", "plain": "ok"}') }); + const table = instance.translations.get().en; + console.log(JSON.stringify({ + loadable: true, + own: Object.getOwnPropertyDescriptor(table, '__proto__') && Object.getOwnPropertyDescriptor(table, '__proto__').value, + plain: table.plain, + keepsPrototype: Object.getPrototypeOf(table) === Object.prototype, + })); + `; + + let result; + try { + // Runs at module evaluation, where jest's per-test timeout does not apply. + result = JSON.parse(execFileSync(process.execPath, ['-e', script], { encoding: 'utf8', timeout: 30000 })); + } catch (error: any) { + throw new Error(`CJS bundle probe failed:\n${error?.stderr || error?.message || error}`); + } + + if (!result.loadable && result.reason !== 'ERR_REQUIRE_ESM') { + throw new Error(`The published CJS entry could not be loaded: ${result.reason}`); + } + + return result; +}; + +const cjs = probeCjs(); + +(cjs.loadable ? describe : describe.skip)('published bundle (CJS entry)', () => { + it('keeps a literal `__proto__` translation key as an own property', () => { + expect(cjs.own).toBe('boom'); + expect(cjs.plain).toBe('ok'); + expect(cjs.keepsPrototype).toBe(true); + }); +}); diff --git a/tests/specs/index.spec.ts b/tests/specs/index.spec.ts index f8cf205..d8f95f5 100644 --- a/tests/specs/index.spec.ts +++ b/tests/specs/index.spec.ts @@ -1,7 +1,7 @@ import { get } from 'svelte/store'; import i18n from '../../src/index'; -import { loggerFactory } from '../../src/logger'; -import { translate } from '../../src/utils'; +import { logger, loggerFactory, setLogger } from '../../src/logger'; +import { read, testRoute, translate } from '../../src/utils'; import { CONFIG, getTranslations } from '../data'; import { filterTranslationKeys } from '../utils'; @@ -9,6 +9,48 @@ const TRANSLATIONS = getTranslations(); const { initLocale = '', loaders = [], parser, log } = CONFIG; +// The library logs through one module-level singleton, so a test that asserts +// on its output has to install a capturing logger. Doing it through this helper +// keeps install and restore symmetric — restoring anything else (a fresh +// factory, CONFIG's level) silently changes the level for every later test. +const withLogger = (level: 'error' | 'warn' | 'debug', impl: any) => { + const previous = logger; + + setLogger(loggerFactory({ level, logger: impl })); + + return () => setLogger(previous); +}; + +// Waits for an observable condition instead of a fixed delay: a load settling +// is not a wall-clock event, and a budget generous enough for a loaded CI +// runner would make the fixed-delay form pointlessly slow everywhere else. The +// deadline stays under jest's own test timeout so a genuine failure reports +// this message rather than jest's. +const delay = (ms: number) => new Promise((resolve) => { setTimeout(resolve, ms); }); + +const until = async (condition: () => boolean, timeout = 2000) => { + const deadline = Date.now() + timeout; + + while (!condition()) { + if (Date.now() > deadline) throw new Error('Timed out waiting for the expected state.'); + + // eslint-disable-next-line no-await-in-loop -- polling is the point + await delay(1); + } +}; + +const captureLogs = (level: 'error' | 'warn' | 'debug') => { + const captured = { error: [] as string[], warn: [] as string[], debug: [] as string[] }; + + const restore = withLogger(level, { + error: (value: any) => captured.error.push(`${value}`), + warn: (value: any) => captured.warn.push(`${value}`), + debug: (value: any) => captured.debug.push(`${value}`), + }); + + return { captured, restore }; +}; + describe('i18n instance', () => { it('exports all properties and methods', () => { const instance = new i18n(); @@ -427,19 +469,389 @@ describe('i18n instance', () => { }); it('handles a locale named like an `Object.prototype` member', async () => { // A loader locale colliding with a prototype member must not throw when the - // `loadedKeys` cache is indexed by it. - const { loadTranslations } = new i18n({ + // `loadedKeys` cache is indexed by it, must keep its data, and must still + // be deduped by that cache. + let calls = 0; + const instance = new i18n({ ...CONFIG, loaders: [ { key: 'common', locale: '__proto__', - loader: async () => ({ greeting: 'Hi' }), + loader: async () => { calls += 1; return { greeting: 'Hi' }; }, }, ], }); - await expect(loadTranslations('__proto__')).resolves.not.toThrow(); + await expect(instance.loadTranslations('__proto__')).resolves.not.toThrow(); + + expect(read(instance.translations.get(), '__proto__')).toEqual( + expect.objectContaining({ 'common.greeting': 'Hi' }), + ); + + // The dedupe cache must hold an OWN entry for the locale — on a plain + // object the write would go through the `__proto__` setter instead, and + // the loader would refire on every route change. + await instance.loadTranslations('__proto__', '/second'); + expect(calls).toBe(1); + }); + it('matches a `g`-flagged route pattern on every navigation', () => { + // `test` advances `lastIndex` on a global/sticky pattern, so a route object + // shared across navigations would match only every other time. + const pattern = /\/shop/g; + + expect(testRoute('/shop')(pattern)).toBe(true); + expect(testRoute('/shop/cart')(pattern)).toBe(true); + expect(testRoute('/shop')(pattern)).toBe(true); + }); + it('leaves a route pattern unmutated', () => { + // The pattern belongs to the consumer's config; matching a route must not + // write state into an object they may also use themselves. + const pattern = /shop/g; + + testRoute('/shop/cart')(pattern); + + expect(pattern.lastIndex).toBe(0); + }); + it('matches a frozen route pattern, including a stateful one', () => { + // A deep-frozen config is legitimate: `lastIndex` is then read-only, so any + // implementation that writes it — directly or through `String.search` — + // throws, and the surrounding catch turns that into a silent permanent + // non-match. + expect(testRoute('/shop')(Object.freeze(/^\/shop/))).toBe(true); + expect(testRoute('/other')(Object.freeze(/^\/shop/))).toBe(false); + expect(testRoute('/shop/cart')(Object.freeze(/\/shop/g))).toBe(true); + + // Sticky still anchors at the start rather than degrading to a free match. + expect(testRoute('/shop/cart')(Object.freeze(/\/shop/y))).toBe(true); + expect(testRoute('/shop/cart')(Object.freeze(/cart/y))).toBe(false); + }); + it('keeps matching a duck-typed route matcher', () => { + // `Loader.Route` is typed `string | RegExp`, but this ships as JS, so + // anything object-shaped is asked for `test` and consumers rely on it. + const matcher = { test: (route: string) => route.startsWith('/shop') }; + + expect(testRoute('/shop/cart')(matcher as any)).toBe(true); + expect(testRoute('/about')(matcher as any)).toBe(false); + + // Its own `test` decides even when it carries pattern-shaped properties: + // copying it would produce `new RegExp(undefined)`, i.e. match everything. + const flagged = { global: true, test: (route: string) => route.startsWith('/shop') }; + + expect(testRoute('/about')(flagged as any)).toBe(false); + expect(testRoute('/shop')({ sticky: true, source: 'nope', test: () => false } as any)).toBe(false); + + // The route reaches a custom matcher unchanged, so it can tell an unset + // route from the string 'undefined'. + expect(testRoute(undefined as any)({ test: (route: any) => route === undefined } as any)).toBe(true); + }); + it('rejects a route that is not a pattern instead of coercing it', () => { + const { captured, restore } = captureLogs('error'); + + let matched: boolean | undefined; + try { + // Coercing this to a regex yields /[object Object]/, which matches any + // route containing one of those characters. + matched = testRoute('/contact')({} as any); + } finally { + restore(); + } + + expect(matched).toBe(false); + expect(captured.error.some((message) => message.includes('Invalid route config!'))).toBe(true); + }); + it('does not report a config error when no route is set yet', async () => { + // `getTranslationProps` runs with `$route === undefined` before the first + // navigation; that is not a broken config. + const { captured, restore } = captureLogs('error'); + + try { + const instance = new i18n({ parser, loaders }); + + await instance.getTranslationProps(initLocale); + } finally { + restore(); + } + + expect(captured.error.filter((message) => message.includes('Invalid route config!'))).toEqual([]); + }); + it('`addTranslations` fails soft on a `null` payload', () => { + const instance = new i18n({ parser, log }); + + // A module that resolved to `null` is consumer input, not a bug in the + // instance: every other step tolerates it, so the whole call must too. + expect(() => instance.addTranslations({ en: null as any, de: { greeting: 'Hallo' } })).not.toThrow(); + + expect(read(instance.translations.get(), 'de')).toEqual( + expect.objectContaining({ greeting: 'Hallo' }), + ); + }); + it('reports a failed load to the caller instead of crashing the process', async () => { + // Errors raised after the fetch (a throwing `preprocess` here) must reach + // whoever awaits the load, rather than being swallowed or surfacing as an + // unhandled rejection from a promise nobody can reach. + const instance = new i18n({ + ...CONFIG, + preprocess: () => { throw new Error('preprocess boom'); }, + loaders: [{ key: 'common', locale: 'en', loader: async () => ({ greeting: 'Hi' }) }], + }); + + await expect(instance.loadTranslations('en', '/')).rejects.toThrow('preprocess boom'); + }); + it('reports a discarded failing load instead of crashing the process', async () => { + // A rejection that escapes fails this test through jest's own handler, so + // the run itself pins the contract; these assertions pin that the failure + // is still reported rather than silently swallowed. + const { captured, restore } = captureLogs('error'); + + try { + const instance = new i18n({ + parser, + preprocess: () => { throw new Error('preprocess boom'); }, + loaders: [{ key: 'common', locale: 'en', loader: async () => ({ greeting: 'Hi' }) }], + }); + + instance.setRoute('/'); + instance.setLocale('en'); + instance.loadTranslations('en', '/second'); + + await until(() => captured.error.some((message) => message.includes('preprocess boom'))); + } finally { + restore(); + } + + expect(captured.error.some((message) => message.includes('preprocess boom'))).toBe(true); + }); + it('reports a failure raised before the load starts', async () => { + // The loader runs as a store subscriber, whose returned promise svelte + // discards. A throw in its synchronous prologue has to reach the caller + // through the same path as a failed fetch, filed under the requested + // locale so `toPromise` does not filter the rejection out. + const brokenLoader: any = { key: 'common', loader: async () => ({ greeting: 'Hi' }) }; + Object.defineProperty(brokenLoader, 'locale', { + enumerable: true, + get: () => { throw new Error('locale getter boom'); }, + }); + + const { captured, restore } = captureLogs('error'); + + try { + const instance = new i18n({ parser, loaders: [brokenLoader] }); + + instance.setRoute('/'); + + await expect(instance.setLocale('en')).rejects.toThrow('locale getter boom'); + } finally { + restore(); + } + + expect(captured.error.some((message) => message.includes('locale getter boom'))).toBe(true); + }); + it('delivers a finished load while another one is still in flight', async () => { + // Characterizes delivery across overlapping loads: the fast route's data + // reaches `t` subscribers while the slow one is still fetching. + let release: (value: unknown) => void = () => {}; + const gate = new Promise((resolve) => { release = resolve; }); + + const instance = new i18n({ + // Returns the text, unlike the suite's key-returning test parser, so the + // assertion below distinguishes "delivered" from "still missing". + parser: { parse: (text: any, _params: any, _locale: any, key: string) => (text === undefined ? key : text) }, + log, + loaders: [ + { key: 'slow', locale: 'en', routes: [/^\/y/], loader: async () => { await gate; return { b: '2' }; } }, + { key: 'fast', locale: 'en', routes: [/^\/x/], loader: async () => ({ a: '1' }) }, + ], + }); + + const seen: string[] = []; + instance.t.subscribe(($t) => seen.push($t('fast.a'))); + + instance.setLocale('en'); + instance.setRoute('/y'); + await instance.setRoute('/x'); + + expect(seen[seen.length - 1]).toBe('1'); + + release({}); + await instance.loading.toPromise(); + }); + it('reports a failing locale lookup from `loadTranslations` as a rejection', async () => { + // The primary public entry point resolves the locale before any promise + // exists, so a throw there would reach the caller synchronously — before + // they could attach the `.catch` the docs tell them to use. + const brokenLoader: any = { key: 'common', loader: async () => ({ greeting: 'Hi' }) }; + Object.defineProperty(brokenLoader, 'locale', { + enumerable: true, + get: () => { throw new Error('locale getter boom'); }, + }); + + const { captured, restore } = captureLogs('error'); + + try { + const instance = new i18n({ parser, loaders: [brokenLoader] }); + + await expect(instance.loadTranslations('en', '/')).rejects.toThrow('locale getter boom'); + } finally { + restore(); + } + + expect(captured.error.some((message) => message.includes('locale getter boom'))).toBe(true); + }); + it('survives a logger it cannot even call', async () => { + // `log.logger` is consumer input: reading the level off it can throw just + // as calling it can. + const restore = withLogger('debug', null); + + try { + const instance = new i18n({ + parser, + loaders: [{ key: 'common', locale: 'en', loader: async () => ({ greeting: 'Hi' }) }], + }); + + instance.setRoute('/'); + + await expect(instance.setLocale('en')).resolves.not.toThrow(); + } finally { + restore(); + } + }); + it('reports a prologue failure under a non-canonical locale', async () => { + // `toPromise` matches on the sanitized locale, so an entry filed under the + // raw input would be filtered out and the caller would see a success. + const brokenLoader: any = { key: 'common', loader: async () => ({ greeting: 'Hi' }) }; + Object.defineProperty(brokenLoader, 'locale', { + enumerable: true, + get: () => { throw new Error('locale getter boom'); }, + }); + + const instance = new i18n({ parser, loaders: [brokenLoader] }); + + instance.setRoute('/'); + + await expect(instance.setLocale('EN')).rejects.toThrow('locale getter boom'); + }); + it('reports a config load that fails before any loader runs', async () => { + // `loadConfig` from the constructor has no caller to reject to. A failure + // in its synchronous section never reaches the loader's handler, so the + // instance would otherwise end up silently half-initialized. + const { captured, restore } = captureLogs('error'); + + try { + // eslint-disable-next-line no-new -- constructing is what is under test + new i18n({ + parser, + preprocess: () => { throw new Error('config boom'); }, + translations: { en: { greeting: 'Hi' } }, + }); + + await until(() => captured.error.some((message) => message.includes('config boom'))); + } finally { + restore(); + } + + expect(captured.error.some((message) => message.includes('config boom'))).toBe(true); + }); + it('survives a logger that throws on every level', async () => { + // A consumer's logger is arbitrary code called from promise handlers that + // nothing awaits; a throw there must not escape as an unhandled rejection. + const throwing = () => { throw new Error('logger boom'); }; + const restore = withLogger('debug', { error: throwing, warn: throwing, debug: throwing }); + + try { + const instance = new i18n({ + parser, + preprocess: () => { throw new Error('preprocess boom'); }, + loaders: [{ key: 'common', locale: 'en', loader: async () => ({ greeting: 'Hi' }) }], + }); + + instance.setRoute('/'); + + // The caller still receives the load's own failure — a throwing logger + // must not replace it, nor escape as an unhandled rejection. + await expect(instance.setLocale('en')).rejects.toThrow('preprocess boom'); + } finally { + restore(); + } + }); + it('does not throw when a route is set before a config is loaded', () => { + const instance = new i18n(); + + // A route can be set while an async `loadConfig` is still pending. + expect(() => instance.setRoute('/')).not.toThrow(); + expect(() => instance.loading.toPromise()).not.toThrow(); + }); + it('purges settled loads without dropping one that started later', async () => { + let releaseB: (value: unknown) => void = () => {}; + let releaseC: (value: unknown) => void = () => {}; + const gateB = new Promise((resolve) => { releaseB = resolve; }); + const gateC = new Promise((resolve) => { releaseC = resolve; }); + + const instance = new i18n({ + parser, + log, + loaders: [ + { key: 'a', locale: 'en', routes: [/^\/a/], loader: async () => ({ a: '1' }) }, + { key: 'b', locale: 'en', routes: [/^\/b/], loader: async () => { await gateB; return { b: '2' }; } }, + { key: 'c', locale: 'en', routes: [/^\/c/], loader: async () => { await gateC; return { c: '3' }; } }, + ], + }); + + instance.setLocale('en'); + + // Settles, so the loading flag drops — the next load's rising edge is what + // runs the purge. + await instance.setRoute('/a'); + expect(instance.loading.get()).toBe(false); + + instance.setRoute('/b'); + + // The purge snapshots the tracked promises when the flag rises, and the + // flag rises a microtask after the route is set. + await until(() => instance.loading.get() === true); + + // `loader` records its entry synchronously, so this one is added after the + // snapshot the purge is waiting on. + instance.setRoute('/c'); + + let resolvedEarly = false; + instance.loading.toPromise().then(() => { resolvedEarly = true; }, () => {}); + + releaseB({}); + + // B's data landing means B settled, so the purge's own wait is over and it + // has deleted everything it snapshotted. + await until(() => !!read(instance.translations.get(), 'en')?.['b.b']); + + // Purging must remove only what it waited for. Clearing the whole set would + // drop the still-running load, and `toPromise()` would report success while + // its data is missing. + expect(resolvedEarly).toBe(false); + + releaseC({}); + await instance.loading.toPromise(); + + expect(read(instance.translations.get(), 'en')).toEqual( + expect.objectContaining({ 'c.c': '3' }), + ); + }); + it('`serialize` keeps a loader locale named like a prototype member', async () => { + // Characterizes the merge: two loaders sharing a prototype-named locale end + // up in one table. Spreading `Object.prototype` yields `{}` either way, so + // this pins the behavior rather than guarding a defect. + const instance = new i18n({ + ...CONFIG, + loaders: [ + { key: 'a', locale: '__proto__', loader: async () => ({ one: '1' }) }, + { key: 'b', locale: '__proto__', loader: async () => ({ two: '2' }) }, + ], + }); + + await instance.loadTranslations('__proto__'); + + expect(read(instance.translations.get(), '__proto__')).toEqual( + expect.objectContaining({ 'a.one': '1', 'b.two': '2' }), + ); }); it('logger works as expected', async () => { const debug = import.meta.jest.spyOn(console, 'debug'); diff --git a/tests/tsconfig.json b/tests/tsconfig.json index b8b1e20..d9983d7 100644 --- a/tests/tsconfig.json +++ b/tests/tsconfig.json @@ -1,6 +1,13 @@ { "extends": "../tsconfig.json", + "compilerOptions": { + // Object spread must stay native here. When tsc downlevels it (below + // es2018) it emits an `Object.assign` helper whose [[Set]] semantics drop + // own '__proto__' keys, so the suite would exercise different + // prototype-pollution behavior than esbuild's output. + "target": "es2020" + }, "include": [ "./" ] -} \ No newline at end of file +} diff --git a/tsconfig.json b/tsconfig.json index 6701cb8..19bd2bc 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,6 +6,9 @@ "outDir": "lib", "sourceMap": false, "strict": true, + // tsup inherits this as its build target, so it sets the published + // bundle's language level. `tests/tsconfig.json` raises it for the suite + // only. "target": "es2017", "moduleResolution": "node", "noEmit": true, diff --git a/tsconfig.tools.json b/tsconfig.tools.json new file mode 100644 index 0000000..abb455f --- /dev/null +++ b/tsconfig.tools.json @@ -0,0 +1,16 @@ +{ + // The root config compiles `src` only — `allowJs` is off there, so the repo's + // own JavaScript config files were never checked by anything. + "extends": "./tsconfig.json", + "compilerOptions": { + "allowJs": true, + "checkJs": true, + "noEmit": true, + // These files import build tooling whose own declarations do not typecheck + // in isolation; the subject here is our config, not their types. + "skipLibCheck": true + }, + "include": [ + "./*.js" + ] +}