[WEB-5048] chore: implements esm exports for all packages #7816
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⛔ Files ignored due to path filters (1)
You can disable this status message by setting the WalkthroughUpdates toolchain and build configs, restructures package exports, and migrates i18n locale data from JSON to TypeScript across multiple languages. Cleans devDependencies and adjusts app package metadata. Tweaks editor TypeScript config and hook return type. Consolidates tsdown options and modifies dependency resolution and overrides. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 |
sriramveeraghanta
force-pushed
the
feat-package-exports
branch
from
6d0a318 to
9464a27
Compare
sriramveeraghanta
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
lifeiscontent
force-pushed
the
feat-package-exports
branch
from
9a85329 to
49de002
Compare
lifeiscontent
force-pushed
the
feat-package-exports
branch
from
49de002 to
f933934
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request implements ESM exports for all packages in the i18n package by converting JSON translation files to TypeScript files with the export default pattern. This change enables proper ESM module support while maintaining type safety.
Key changes:
- Converted JSON translation files to TypeScript
.tsfiles with proper ESM exports - Added
as constassertions to maintain type safety and literal types - Removed corresponding JSON files that are no longer needed
Reviewed Changes
Copilot reviewed 50 out of 162 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/i18n/src/locales/*/editor.ts | Created empty TS export files replacing JSON files |
| packages/i18n/src/locales/*/accessibility.ts | Converted accessibility translations from JSON to TS with ESM exports |
| packages/i18n/src/locales/es/translations.ts | Converted Spanish translations from JSON to TS with ESM exports |
| packages/i18n/src/locales/en/translations.ts | Converted English translations from JSON to TS with ESM exports |
| packages/i18n/src/locales/en/core.ts | Converted English core translations from JSON to TS with ESM exports |
| packages/i18n/src/locales/*/*.json | Removed original JSON files that have been replaced by TS files |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/hooks/tsdown.config.ts (1)
3-11: Restore externalization of peer dependencies.The removal of
external: ["react", "react-dom"](as mentioned in the AI summary) will cause these peer dependencies to be bundled into the output, leading to:
- Increased bundle size
- Potential React version conflicts
- Multiple React instances in consuming applications
Libraries should externalize peer dependencies to ensure they're provided by the consuming application.
Add the
externalfield back to the configuration:export default defineConfig({ entry: ["src/index.ts"], outDir: "dist", format: ["esm", "cjs"], + external: ["react", "react-dom"], exports: true, dts: true, clean: true, sourcemap: true, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
.mise.toml(1 hunks)apps/admin/next.config.js(1 hunks)apps/admin/package.json(0 hunks)apps/live/package.json(1 hunks)apps/live/tsdown.config.ts(1 hunks)apps/space/next.config.js(1 hunks)apps/space/package.json(0 hunks)apps/web/next.config.js(1 hunks)apps/web/package.json(1 hunks)package.json(1 hunks)packages/constants/package.json(1 hunks)packages/constants/tsdown.config.ts(1 hunks)packages/decorators/package.json(2 hunks)packages/decorators/tsdown.config.ts(1 hunks)packages/editor/package.json(2 hunks)packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts(1 hunks)packages/editor/tsconfig.json(1 hunks)packages/editor/tsdown.config.ts(1 hunks)packages/hooks/package.json(2 hunks)packages/hooks/tsdown.config.ts(1 hunks)packages/i18n/package.json(2 hunks)packages/i18n/src/locales/cs/accessibility.json(0 hunks)packages/i18n/src/locales/cs/accessibility.ts(1 hunks)packages/i18n/src/locales/cs/editor.json(0 hunks)packages/i18n/src/locales/cs/editor.ts(1 hunks)packages/i18n/src/locales/cs/translations.json(0 hunks)packages/i18n/src/locales/cs/translations.ts(1 hunks)packages/i18n/src/locales/de/accessibility.json(0 hunks)packages/i18n/src/locales/de/accessibility.ts(1 hunks)packages/i18n/src/locales/de/editor.json(0 hunks)packages/i18n/src/locales/de/editor.ts(1 hunks)packages/i18n/src/locales/de/translations.json(0 hunks)packages/i18n/src/locales/de/translations.ts(1 hunks)packages/i18n/src/locales/en/accessibility.json(0 hunks)packages/i18n/src/locales/en/accessibility.ts(1 hunks)packages/i18n/src/locales/en/core.json(0 hunks)packages/i18n/src/locales/en/core.ts(1 hunks)packages/i18n/src/locales/en/editor.json(0 hunks)packages/i18n/src/locales/en/editor.ts(1 hunks)packages/i18n/src/locales/en/translations.json(0 hunks)packages/i18n/src/locales/en/translations.ts(1 hunks)packages/i18n/src/locales/es/accessibility.json(0 hunks)packages/i18n/src/locales/es/accessibility.ts(1 hunks)packages/i18n/src/locales/es/editor.json(0 hunks)packages/i18n/src/locales/es/editor.ts(1 hunks)packages/i18n/src/locales/es/translations.ts(1 hunks)packages/i18n/src/locales/fr/accessibility.json(0 hunks)packages/i18n/src/locales/fr/accessibility.ts(1 hunks)packages/i18n/src/locales/fr/editor.json(0 hunks)packages/i18n/src/locales/fr/editor.ts(1 hunks)
💤 Files with no reviewable changes (16)
- apps/space/package.json
- packages/i18n/src/locales/fr/accessibility.json
- packages/i18n/src/locales/fr/editor.json
- packages/i18n/src/locales/cs/translations.json
- packages/i18n/src/locales/cs/accessibility.json
- apps/admin/package.json
- packages/i18n/src/locales/de/translations.json
- packages/i18n/src/locales/de/editor.json
- packages/i18n/src/locales/en/accessibility.json
- packages/i18n/src/locales/en/core.json
- packages/i18n/src/locales/de/accessibility.json
- packages/i18n/src/locales/es/accessibility.json
- packages/i18n/src/locales/es/editor.json
- packages/i18n/src/locales/cs/editor.json
- packages/i18n/src/locales/en/translations.json
- packages/i18n/src/locales/en/editor.json
🔇 Additional comments (19)
packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts (2)
10-11: LGTM! Type-only imports support ESM tree-shaking.The
typekeyword ensures these imports are stripped at compile time, improving ESM bundle optimization.
19-25: Explicit return type improves ESM compatibility and type safety.The
TReturntype definition and explicit function return type provide:
- Clearer API contract for consumers
- Better type checking and IDE support
- Improved ESM export resolution
The type definition accurately matches the implementation.
.mise.toml (1)
1-2: LGTM!The Node version specification aligns with the
enginesrequirement in the root package.json and ensures consistent tooling across the development environment.package.json (2)
39-46: Correctly configured onlyBuiltDependencies.The
onlyBuiltDependenciesfield properly lists native/compiled packages that require building from source. This configuration is appropriate for the listed dependencies.
37-37: Confirm Vite version pinning: Vite 7.0.7 is a valid release but lags the current stable (7.1.9). Verify that pinning to 7.0.7 aligns with your stability and security requirements.apps/live/package.json (1)
20-20: LGTM!The author metadata has been properly populated. This is a straightforward metadata improvement with no functional impact.
packages/i18n/src/locales/fr/editor.ts (1)
1-1: LGTM!The empty const object serves as a type-safe placeholder for French editor locale resources. This pattern is consistent with the migration from JSON to TypeScript modules across all locales.
packages/i18n/src/locales/en/editor.ts (1)
1-1: LGTM!Consistent with the other locale modules. The empty const object provides a type-safe placeholder for English editor locale resources during the JSON-to-TypeScript migration.
packages/i18n/src/locales/de/editor.ts (1)
1-1: LGTM!The German editor locale placeholder follows the established pattern and provides a type-safe foundation for future translation additions.
packages/i18n/src/locales/es/editor.ts (1)
1-1: LGTM!The Spanish editor locale placeholder completes the consistent migration pattern across all locale modules (en, de, es, fr), providing a unified type-safe approach for future locale content.
packages/i18n/package.json (2)
7-13: LGTM! Clean ESM/CJS dual export setup.The exports field correctly maps ESM (import) to
.mjsand CJS (require) to.js, with an explicit package.json export for tooling compatibility.
41-43: LGTM! Bottom-level declarations provide legacy compatibility.The bottom-level main/module/types fields align with the exports field and provide fallback support for older tooling.
packages/i18n/src/locales/en/accessibility.ts (1)
1-34: LGTM! Well-structured accessibility labels.The migration from JSON to TypeScript with
as constprovides better type safety and IDE support while maintaining the same data structure.packages/i18n/src/locales/en/core.ts (1)
1-172: LGTM! Comprehensive core translations with type safety.The TypeScript module approach with
as constprovides better maintainability and type checking compared to the previous JSON format.packages/i18n/src/locales/fr/accessibility.ts (1)
1-34: LGTM! French accessibility labels follow the established pattern.The structure mirrors the English version, maintaining consistency across locales.
packages/constants/package.json (1)
27-36: LGTM! Consistent ESM/CJS export structure.The exports field and bottom-level declarations follow the same pattern as the i18n package, providing clean dual-format support.
packages/editor/tsconfig.json (1)
2-7: LGTM! Modern TypeScript configuration for bundler consumers.The
"moduleResolution": "Bundler"setting is appropriate for a library package that will be consumed by modern bundlers, andES2022provides a current module target.packages/editor/tsdown.config.ts (2)
15-15: Clean flag enabled—verify CI and build workflows.Switching
cleanfromfalsetotruewill delete the output directory before each build. This is generally good practice, but ensure that CI pipelines or local development workflows don't rely on incremental builds or persistent artifacts in thedistdirectory.Additionally, the AI summary indicates that
minify: truewas removed. If minification is still desired for production builds, consider re-enabling it or documenting the rationale for its removal.
7-13: Verify copied styles output After running the build, confirm thatdist/styles/index.cssexists (notdist/src/styles/index.css) to ensure the"./styles": "./dist/styles/index.css"export is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/editor/package.json (1)
13-18: Add explicittypesconditions to exports.The exports for
"."and"./lib"lack explicittypesconditions. With"type": "module"and a top-level"types": "./dist/index.d.cts"(line 10), TypeScript will fall back to CJS type declarations even for ESM consumers, causing module resolution mismatches.This issue was already flagged in a previous review. The recommended fix is to add nested
typesconditions to both"import"and"require"entries, ensuring ESM consumers get.d.tsand CJS consumers get.d.cts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/editor/package.json(1 hunks)packages/editor/tsdown.config.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/editor/tsdown.config.ts (2)
8-13: LGTM!The exports configuration correctly adds the styles export, matching the package.json export at line 21 in
packages/editor/package.json.
15-15: LGTM!Setting
clean: trueensures the dist directory is cleaned before each build, preventing stale artifacts.packages/editor/package.json (2)
7-7: LGTM!Adding
"type": "module"correctly declares this as an ESM package, which aligns with the dual ESM/CJS output strategy (.js for ESM, .cjs for CommonJS).
20-20: LGTM!Adding the
"./package.json"export is a best practice that allows consumers to explicitly access package metadata.
Description
Type of Change
Summary by CodeRabbit
New Features
Refactor
Chores