Feature/format#53
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR bumps Tempo/library/root package versions to 3.5.2, adds a ChangesTempo 3.5.2 release changes
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant TempoBundle as tempo.bundle.min.js
participant PluginBundle as community plugin bundle
participant Magma as window.Magma
Browser->>TempoBundle: load script
TempoBundle->>Magma: attach Tempo core
Browser->>PluginBundle: load script
PluginBundle->>Magma: attach plugins
Browser->>Magma: extract { Tempo, plugins }
Browser->>Browser: Tempo.extend(plugins.astro)
sequenceDiagram
participant CatalogList
participant Firestore as Firestore REST endpoint
participant EcosystemPage as ecosystem.md
EcosystemPage->>CatalogList: render <CatalogList />
CatalogList->>Firestore: fetch plugin documents on mount
Firestore-->>CatalogList: document fields
CatalogList->>CatalogList: map to Plugin list, filter by status/price
CatalogList-->>EcosystemPage: render community/premium/coming-soon sections
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/tempo/src/plugin-api.index.ts (1)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant named re-export.
Line 18's
export { defineTerm, defineRange, getTermRange } from './plugin/term/term.index.js';duplicates what line 17'sexport * from './plugin/term/term.index.js';already re-exports. Runtime-wise this is harmless (explicit named exports simply shadow the star export for the same names), but it's dead weight that can confuse readers about the intended public surface.♻️ Suggested cleanup
export * from './plugin/term/term.index.js'; -export { defineTerm, defineRange, getTermRange } from './plugin/term/term.index.js';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/src/plugin-api.index.ts` around lines 17 - 18, The plugin API entrypoint has a redundant named re-export because plugin-api.index.ts already uses export * from './plugin/term/term.index.js' to expose defineTerm, defineRange, and getTermRange. Clean up the public surface by removing the explicit named export line and keep only the star re-export, or otherwise make the export strategy consistent so plugin-api.index.ts does not duplicate the same symbols.packages/tempo/src/module/module.format.ts (1)
313-352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist duplicated
dtOptionscomputation.
const dtOptions = config?.intl?.dateTimeFormat ?? {};is recomputed identically in thelocale,zzzz, andzzzzzcases (lines 314, 341, 348). Since it doesn't depend onmodortoken, hoisting it once above the modifierforloop avoids the repeated lookup/allocation per modifier evaluated.♻️ Suggested refactor
+ const dtOptions = config?.intl?.dateTimeFormat ?? {}; for (const mod of modifiers) { switch (mod.toLowerCase()) { ... case 'locale': { try { if (token.startsWith('#') && isTempo(obj)) { ... } else { - const dtOptions = config?.intl?.dateTimeFormat ?? {}; const tzOpts = { ...dtOptions, timeZone: zdt.timeZoneId, calendar: zdt.calendarId }; ... case 'zzzz': if (token === 'tz') { - const dtOptions = config?.intl?.dateTimeFormat ?? {}; const parts = getDTF(config?.locale, { ...dtOptions, timeZone: zdt.timeZoneId, timeZoneName: 'short' }).formatToParts(zdt.epochMilliseconds); ... case 'zzzzz': if (token === 'tz') { - const dtOptions = config?.intl?.dateTimeFormat ?? {}; const parts = getDTF(config?.locale, { ...dtOptions, timeZone: zdt.timeZoneId, timeZoneName: 'long' }).formatToParts(zdt.epochMilliseconds); ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/src/module/module.format.ts` around lines 313 - 352, Hoist the repeated dtOptions lookup in the formatting switch so it is computed once instead of inside each locale/timezone branch. In module.format.ts, move const dtOptions = config?.intl?.dateTimeFormat ?? {} to the shared scope above the modifier loop in the formatter logic, then reuse it in the locale, zzzz, and zzzzz cases; this keeps behavior unchanged while removing duplicated work.packages/tempo/src/module/module.duration.ts (1)
213-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the repeated option-resolution chain.
Lines 213-214 duplicate the same 7-step fallback pattern (
rtOptions?.X || rtConfig?.X || opts['intl']?.relativeTimeFormat?.X || opts['rtfX'] || this.config...) forstyleand nownumeric. Extracting a small helper would reduce duplication and make future option additions (e.g. locale overrides) easier to maintain.♻️ Suggested refactor
+ const resolveRtfOption = <T,>(key: 'style' | 'numeric', fallback: T): T => + rtOptions?.[key] ?? rtConfig?.[key] ?? opts['intl']?.relativeTimeFormat?.[key] + ?? opts[`rtf${key[0].toUpperCase()}${key.slice(1)}`] + ?? (this as any).config.intl?.relativeTimeFormat?.[key] + ?? (this as any).config[`rtf${key[0].toUpperCase()}${key.slice(1)}`] + ?? fallback; + const getFormatted = (val: number, u: any) => { const su = singular(u); if (isFunction(rtf)) return rtf(val, su); if (rtf instanceof Intl.RelativeTimeFormat) return rtf.format(val, su); - const style = rtOptions?.style || rtConfig?.style || opts['intl']?.relativeTimeFormat?.style || opts['rtfStyle'] || (this as any).config.intl?.relativeTimeFormat?.style || (this as any).config['rtfStyle'] || 'narrow'; - const numeric = rtOptions?.numeric || rtConfig?.numeric || opts['intl']?.relativeTimeFormat?.numeric || opts['rtfNumeric'] || (this as any).config.intl?.relativeTimeFormat?.numeric || (this as any).config['rtfNumeric'] || 'always'; + const style = resolveRtfOption('style', 'narrow'); + const numeric = resolveRtfOption('numeric', 'always'); return getRelativeTime(val, su as Intl.RelativeTimeFormatUnit, locale, style, numeric); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/src/module/module.duration.ts` around lines 213 - 215, The relative-time option resolution in module.duration.ts repeats the same fallback chain for both style and numeric, so extract that logic into a small helper and reuse it from the duration formatting path. Use the existing symbols in this area, especially the getRelativeTime call and the rtOptions/rtConfig/opts resolution flow, so future option additions only need one place to update. Keep the helper focused on resolving a single option key from the same precedence order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/tempo/src/module/module.duration.ts`:
- Around line 213-215: The relative-time option resolution in module.duration.ts
repeats the same fallback chain for both style and numeric, so extract that
logic into a small helper and reuse it from the duration formatting path. Use
the existing symbols in this area, especially the getRelativeTime call and the
rtOptions/rtConfig/opts resolution flow, so future option additions only need
one place to update. Keep the helper focused on resolving a single option key
from the same precedence order.
In `@packages/tempo/src/module/module.format.ts`:
- Around line 313-352: Hoist the repeated dtOptions lookup in the formatting
switch so it is computed once instead of inside each locale/timezone branch. In
module.format.ts, move const dtOptions = config?.intl?.dateTimeFormat ?? {} to
the shared scope above the modifier loop in the formatter logic, then reuse it
in the locale, zzzz, and zzzzz cases; this keeps behavior unchanged while
removing duplicated work.
In `@packages/tempo/src/plugin-api.index.ts`:
- Around line 17-18: The plugin API entrypoint has a redundant named re-export
because plugin-api.index.ts already uses export * from
'./plugin/term/term.index.js' to expose defineTerm, defineRange, and
getTermRange. Clean up the public surface by removing the explicit named export
line and keep only the star re-export, or otherwise make the export strategy
consistent so plugin-api.index.ts does not duplicate the same symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 40d69a98-2f60-4bb0-849c-6776706a68d8
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/tempo/public/plugin-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (23)
.editorconfigpackage.jsonpackages/library/package.jsonpackages/library/src/common/international.library.tspackages/tempo/.vitepress/config.tspackages/tempo/.vitepress/theme/components/CatalogList.vuepackages/tempo/.vitepress/theme/index.tspackages/tempo/CHANGELOG.mdpackages/tempo/README.mdpackages/tempo/doc/ecosystem.mdpackages/tempo/doc/installation.mdpackages/tempo/doc/releases/index.mdpackages/tempo/doc/releases/v3.x.mdpackages/tempo/doc/tempo.format.mdpackages/tempo/package.jsonpackages/tempo/rollup.config.jspackages/tempo/src/module/module.duration.tspackages/tempo/src/module/module.format.tspackages/tempo/src/plugin-api.index.tspackages/tempo/src/plugin/plugin.index.tspackages/tempo/src/tempo.entry.tspackages/tempo/src/tempo.index.tspackages/tempo/src/tempo.version.ts
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation