[WEB-4810] feat: migrate to tsdown from tsup#7679
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the build toolchain from tsup to tsdown across multiple packages and applications. The migration involves replacing tsup configuration files with tsdown equivalents, updating package.json dependencies, and modifying build scripts.
- Replace tsup with tsdown as the build tool across all packages
- Update configuration files to use tsdown's simpler configuration structure
- Modify npm scripts to use tsdown commands instead of tsup
Reviewed Changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/*/tsup.config.ts | Remove tsup configuration files |
| packages/*/tsdown.config.ts | Add new tsdown configuration files with basic settings |
| packages/*/package.json | Update dependencies from tsup to tsdown and modify build scripts |
| apps/live/tsup.config.ts | Remove tsup configuration file |
| apps/live/tsdown.config.ts | Add new tsdown configuration file |
| apps/live/tsconfig.json | Update TypeScript config to include tsdown.config.ts instead of tsup.config.ts |
| apps/live/package.json | Update live app dependencies and scripts |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughTooling migrated from tsup to tsdown across apps and packages. Added tsdown config files, removed tsup configs, and updated scripts and workspace catalog. i18n package now builds to dist with new exports, a locales registry with dynamic imports, and adjusted store loading logic. Some package entry fields updated to dist outputs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as Consumer
participant Store as TranslationStore
participant Registry as locales (registry)
participant Loader as Dynamic Import
UI->>Store: load(language, files)
Store->>Registry: get locales[language]
alt language found
Store->>Registry: filter available file keys
loop for each fileKey
Store->>Loader: localeData[fileKey]() // dynamic import
Loader-->>Store: module (JSON default)
end
Store->>Store: merge modules.default into result
Store-->>UI: merged translations
else language missing
Store-->>UI: throw Error("Locale not found")
end
note over Store: New flow uses registry and guarded loading
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/package.json (1)
12-22: Change type paths to.d.tsin packages/editor/package.jsonpackages/editor/package.json 12c12 - "types": "./dist/index.d.mts", + "types": "./dist/index.d.ts", 15c15 - "types": "./dist/index.d.mts", + "types": "./dist/index.d.ts", 20c20 - "types": "./dist/lib.d.mts", + "types": "./dist/lib.d.ts",After rebuilding, verify that
dist/index.d.tsanddist/lib.d.tsare emitted.
♻️ Duplicate comments (2)
packages/editor/package.json (1)
25-25: Clarification on the prior comment:--minifyis supported by tsdown.No action needed regarding the
--minifyflag; it’s a valid CLI option. (tsdown.dev)apps/live/package.json (1)
10-11: Dev workflow regression: bring back auto-serve and align output filename/formatSwitching to
tsdown --watchdrops the previous auto-start behavior, breaking the DX noted earlier. Also, confirm thattsdownactually emitsdist/server.js(ESM vs CJS) to match thestartscript; with"type": "module", a CJSdist/server.jswill fail at runtime.Apply this to restore the dev loop (build+serve) and watch all plausible output extensions:
"scripts": { - "dev": "tsdown --watch", + "dev": "concurrently -k -n \"BUILD,SERVE\" -c \"yellow,cyan\" \"tsdown --watch\" \"nodemon --watch dist --ext js,mjs,cjs --env-file=.env dist/server.js\"", "build": "tsc --noEmit && tsdown", "start": "node --env-file=.env dist/server.js",Action items:
- If
tsdownemits ESM asserver.mjs, updatestart(and the nodemon target) todist/server.mjs; if it emits CJS asserver.cjs, either changestartto that file or set"type": "commonjs"for this package.Optional: declare Node engine to ensure
--env-filesupport.+ "engines": { "node": ">=20.6.0" },
🧹 Nitpick comments (10)
apps/live/tsconfig.json (1)
24-24: Including tsdown.config.ts in TSC input: verify intentIncluding the bundler config in "include" will type-check it and require tsdown types to be resolvable from this workspace. If you don’t want config files type-checked, drop it from include and keep it excluded. Otherwise, ensure tsdown is installed for this app and Node ≥ 20.19 in CI/tooling. (tsdown.dev)
packages/decorators/package.json (1)
14-15: Optional: move flags into config to keep scripts stableConsider moving minify and other options to tsdown.config.ts so the script remains simply
tsdown. This reduces CLI/config drift across packages. (tsdown.dev)packages/hooks/package.json (1)
14-15: Optional: add type-checking step for consistencySome packages run
tsc --noEmit && tsdownto fail fast on types. Consider aligning here, unless you rely solely on tsdown’s DTS pass. (tsdown.dev)packages/utils/package.json (1)
14-15: Optional: align minification behaviorOther packages minify in build; consider enabling minification here (via config) for consistent output size across libraries. (tsdown.dev)
packages/types/tsdown.config.ts (1)
3-7: If this package is types-only, avoid emitting JSFor a pure “types” package, emitting JS bundles is unnecessary. Consider a types-only build and publishing only .d.ts.
Option (illustrative): use tsdown’s DTS-only mode if available, or rely on tsc with emitDeclarationOnly and skip bundling. Example config (outside diff):
import { defineConfig } from "tsdown"; export default defineConfig({ entry: ["src/index.ts"], outDir: "dist", dts: true, // omit JS output if tsdown supports a DTS-only toggle; otherwise keep format minimal. format: [], });packages/ui/package.json (2)
15-16: Wire sourcemaps/clean via CLI (until config includes them)Keeps builds reproducible and artifacts debuggable.
- "build": "tsc --noEmit && tsdown", - "dev": "tsdown --watch", + "build": "tsc --noEmit && tsdown --clean --sourcemap", + "dev": "tsdown --watch --sourcemap",
15-16: Add exports map to avoid dual-package hazardImproves Node/ bundler resolution and TypeScript type mapping.
Example (outside diff; place at top level):
"exports": { ".": { "types": "./dist/index.d.ts", "import": "./dist/index.mjs", "require": "./dist/index.js" }, "./package.json": "./package.json" }packages/utils/tsdown.config.ts (1)
1-7: Config is fine; consider opt-ins for DX/build hygiene (optional).If supported, enable source maps in watch builds and ensure deps are not accidentally bundled (externals), to keep artifacts slim and debuggable.
apps/live/tsdown.config.ts (1)
1-7: Server bundle: consider explicit Node target/platform and source maps (optional).Explicit platform/target (e.g., node18+) and sourcemaps help runtime correctness and debugging for the live app.
packages/constants/tsdown.config.ts (1)
1-7: Parity with previous tsup settings: consider enabling dts/minify/clean.
- DTS can be auto-enabled by a "types" field; set dts: true if you want it explicit. (tsdown.dev)
- Minification is supported via CLI or config; if tsup previously minified, add it here. (tsdown.dev)
- Cleaning the output dir can be configured (defaults target to the outDir when enabled). (tsdown.dev)
import { defineConfig } from "tsdown"; export default defineConfig({ entry: ["src/index.ts"], outDir: "dist", format: ["esm", "cjs"], + dts: true, + minify: true, + clean: true, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
apps/live/package.json(2 hunks)apps/live/tsconfig.json(1 hunks)apps/live/tsdown.config.ts(1 hunks)apps/live/tsup.config.ts(0 hunks)packages/constants/package.json(2 hunks)packages/constants/tsdown.config.ts(1 hunks)packages/constants/tsup.config.ts(0 hunks)packages/decorators/package.json(2 hunks)packages/decorators/tsdown.config.ts(1 hunks)packages/decorators/tsup.config.ts(0 hunks)packages/editor/package.json(2 hunks)packages/editor/tsdown.config.ts(1 hunks)packages/editor/tsup.config.ts(0 hunks)packages/hooks/package.json(2 hunks)packages/hooks/tsdown.config.ts(1 hunks)packages/hooks/tsup.config.ts(0 hunks)packages/logger/package.json(2 hunks)packages/logger/tsdown.config.ts(1 hunks)packages/logger/tsup.config.ts(0 hunks)packages/services/package.json(2 hunks)packages/services/tsdown.config.ts(1 hunks)packages/services/tsup.config.ts(0 hunks)packages/types/package.json(2 hunks)packages/types/tsdown.config.ts(1 hunks)packages/ui/package.json(2 hunks)packages/ui/tsdown.config.ts(1 hunks)packages/ui/tsup.config.ts(0 hunks)packages/utils/package.json(2 hunks)packages/utils/tsdown.config.ts(1 hunks)packages/utils/tsup.config.ts(0 hunks)
💤 Files with no reviewable changes (9)
- packages/services/tsup.config.ts
- packages/utils/tsup.config.ts
- packages/decorators/tsup.config.ts
- packages/logger/tsup.config.ts
- packages/ui/tsup.config.ts
- packages/hooks/tsup.config.ts
- apps/live/tsup.config.ts
- packages/editor/tsup.config.ts
- packages/constants/tsup.config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
PR: makeplane/plane#7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
packages/hooks/package.jsonpackages/constants/package.jsonpackages/logger/package.jsonpackages/services/package.jsonpackages/types/package.json
🔇 Additional comments (12)
packages/decorators/package.json (1)
14-15: CLI flags look valid with tsdownBoth --minify and --watch are supported by tsdown; keeping them in scripts is fine. (tsdown.dev)
packages/hooks/package.json (2)
14-15: CLI flags are correct for tsdown--minify and --watch are supported; the scripts are fine. (tsdown.dev)
31-31: tsdown version likely does not exist^0.14.2 isn’t published; use a published range like ^0.13.0. (npmjs.com)
- "tsdown": "^0.14.2", + "tsdown": "^0.13.0",Likely an incorrect or invalid review comment.
packages/utils/package.json (1)
14-15: Build/dev scripts LGTMSwitch to
tsdownand watch mode looks correct.packages/services/package.json (1)
8-9: Build/dev scripts: migration looks fine
tsc --noEmit && tsdownand--watchmatch tsdown’s CLI. (tsdown.dev)packages/ui/package.json (2)
15-16: Dist filenames align with main/module/types tsdown.config.ts is configured for both ESM and CJS, and package.json’s main, module, and types correctly reference./dist/index.js,./dist/index.mjs, and./dist/index.d.ts. Build locally to verify those artifacts are emitted.
78-78: tsdown version aligned and Node engine spec validated All packages use tsdown ^0.14.2 and the declared engines.node (>=22.18.0) exceeds tsdown’s minimum requirement (>=20.19.0) (tsdown.dev)packages/constants/package.json (1)
37-37: No action needed:tsdownis consistently set to^0.14.2across all packages
Allpackage.jsonfiles inpackages/**andapps/liveuse the same^0.14.2version.packages/decorators/tsdown.config.ts (1)
1-7: LGTM.Minimal config is consistent with the migration pattern.
packages/logger/package.json (1)
21-22: Pin Node ≥20.19 in CI and confirm externals are externalized
- Add a
with: node-version: '20.19'(or newer) under eachactions/setup-node@v4step in.github/workflows/*.yml.- Run a local build and inspect the
dist/output to verifywinston(and other deps) remain external and aren’t bundled.packages/types/package.json (1)
20-22: Verify ESM/CJS entry filenames
After runningnpm run build, confirm that themoduleandexports.importfields in packages/types/package.json point to the actual output file extensions (e.g., .mjs/.cjs if tsdown emits those); update them if they diverge.apps/live/package.json (1)
61-61: Verified complete migration off tsup
Notsupreferences found andtsdown.config.tsexists inapps/live.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/hooks/package.json (1)
7-9: Fix main field: tsdown emits CJS as .cjs by defaultWith tsdown, CJS builds default to .cjs. Your "main" points to ./dist/index.js, which will likely break require() consumers unless you’ve customized outExtensions. Either:
- Switch main to ./dist/index.cjs, or
- Configure outExtensions/fixedExtension, or
- Let tsdown auto-generate exports/main/module/types via exports: true in tsdown.config.ts. (tsdown.dev)
Apply this minimal fix if you’re using defaults:
- "main": "./dist/index.js", + "main": "./dist/index.cjs",
♻️ Duplicate comments (1)
packages/hooks/package.json (1)
14-15: Migration to tsdown: good; add Node engines and keep type generation fast
- Build/dev scripts look correct; --watch is supported. (tsdown.dev)
- Add engines to enforce Node ≥ 20.19, required by tsdown. (tsdown.dev)
- d.ts emission is auto-enabled because you already have a "types" field; consider enabling isolatedDeclarations for faster DTS. (tsdown.dev)
Add engines (outside the changed lines):
"private": true, + "engines": { "node": ">=20.19" },Optional tsconfig tweak (outside this file) for speed:
- compilerOptions.isolatedDeclarations: true. (tsdown.dev)
🧹 Nitpick comments (1)
packages/hooks/package.json (1)
31-31: Dev dep update LGTM; optionally enable package sanity checks
- Adding tsdown ^0.14.2 is fine.
- Optional: add publint in CI via tsdown --publint to catch package.json export/type issues during the build. (tsdown.dev)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/live/package.json(2 hunks)packages/decorators/package.json(2 hunks)packages/editor/package.json(2 hunks)packages/hooks/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/live/package.json
- packages/decorators/package.json
- packages/editor/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
PR: makeplane/plane#7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
packages/hooks/package.json
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
sriramveeraghanta
changed the title
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
sriramveeraghanta
requested review from
aaryan610,
lifeiscontent and
prateekshourya29
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
packages/propel/package.json (1)
7-8: tsdown migration looks good; confirm the built dist is actually consumed
tsdownbuilds to dist, but this package’sexportsstill point atsrc/*. If dist artifacts are intended for consumption, alignexportsto dist; otherwise consider whether the dist build is needed.packages/i18n/package.json (1)
25-27: Make react a peer dependency to avoid duplicate React copiesLibraries should not hard-depend on React; let the app provide it.
Apply:
"dependencies": { "@plane/utils": "workspace:*", "intl-messageformat": "^10.7.11", "mobx": "^6.13.5", "mobx-react": "^9.1.0", - "lodash": "^4.17.21", - "react": "^18.3.1" + "lodash": "^4.17.21" }, + "peerDependencies": { + "react": ">=18 <19" + }, + "devDependencies": { + "@plane/eslint-config": "workspace:*", + "@plane/typescript-config": "workspace:*", + "@types/node": "^22.5.4", + "@types/lodash": "4.17.20", + "@types/react": "^18.3.11", + "typescript": "5.8.3", + "tsdown": "^0.14.2", + "react": "^18.3.1" + }packages/propel/tsdown.config.ts (2)
4-17: Align entries with subpath exports (charts)You build
src/charts/index.tsbutpackage.jsonexports./charts/*subpaths. If consumers import@plane/propel/charts/<name>, ensure either:
- dist has matching per-chart entries, or
exportsalso includes"./charts": "./src/charts/index.ts".Example addition to package.json (outside changed range):
{ "exports": { "./charts": "./src/charts/index.ts" } }
18-21: Nice config; consider externals and sourcemapsAvoid bundling app deps and improve DX.
outDir: "dist", format: ["esm", "cjs"], dts: true, + external: ["react", "react-dom"], + sourcemap: true,packages/i18n/src/store/index.ts (5)
19-21: Type safety for coreTranslations entry.If ITranslations expects a nested dictionary type, enCore should align with it. If it’s loosely typed (any), consider tightening to a TranslationObject alias to avoid cascading any.
Apply:
- private coreTranslations: ITranslations = { + type TranslationObject = Record<string, unknown>; + private coreTranslations: ITranslations = { en: enCore, };
103-112: Avoid attempting to load languages not present in locales registry.If SUPPORTED_LANGUAGES contains a code missing from locales, importAndMergeFiles throws. Filter by registry to reduce noise and wasted work.
- const remainingLanguages = SUPPORTED_LANGUAGES.map((lang) => lang.value).filter( - (lang) => - !this.loadedLanguages.has(lang as TLanguage) && lang !== this.currentLocale && lang !== FALLBACK_LANGUAGE - ); + const registryLangs = new Set(Object.keys(locales) as TLanguage[]); + const remainingLanguages = SUPPORTED_LANGUAGES.map((l) => l.value as TLanguage).filter( + (lang) => + registryLangs.has(lang) && + !this.loadedLanguages.has(lang) && + lang !== this.currentLocale && + lang !== FALLBACK_LANGUAGE + );
118-131: Skip re-importing “core” when it’s already bundled in coreTranslations.For en, core is already eagerly included via enCore. You’re importing it again through importAndMergeFiles, doing redundant work and risking divergence.
private async importLanguageFile(language: TLanguage) { - const files = Object.values(ETranslationFiles); - return this.importAndMergeFiles(language, files); + const all = Object.values(ETranslationFiles) as string[]; + const files = + this.coreTranslations[language] ? all.filter((f) => f !== ETranslationFiles.core) : all; + return this.importAndMergeFiles(language, files); }
139-160: Remove anys and narrow dynamic import types.The reducer and modules are typed as any; this trips lint and loses safety.
- private async importAndMergeFiles(language: TLanguage, files: string[]) { + private async importAndMergeFiles(language: TLanguage, files: string[]) { + type TranslationObject = Record<string, unknown>; + type JSONModule = { default: TranslationObject }; try { const localeData = locales[language as keyof typeof locales]; if (!localeData) { throw new Error(`Locale data not found for language: ${language}`); } // Filter out files that don't exist for this language const availableFiles = files.filter((file) => { const fileKey = file as keyof typeof localeData; return fileKey in localeData; }); - const importPromises = availableFiles.map((file) => { - const fileKey = file as keyof typeof localeData; - return localeData[fileKey](); - }); + const importPromises: Array<Promise<JSONModule>> = availableFiles.map((file) => { + const fileKey = file as keyof typeof localeData; + return (localeData[fileKey] as () => Promise<JSONModule>)(); + }); const modules = await Promise.all(importPromises); - const merged = modules.reduce((acc: any, module: any) => merge(acc, module.default), {}); - return { default: merged }; + const merged = modules.reduce<TranslationObject>((acc, module) => merge(acc, module.default), {}); + return { default: merged }; } catch (error) { throw new Error(`Failed to import and merge files for ${language}: ${error}`); } }
103-112: Background loading strategy: consider demand-driven loading.Eagerly fetching all locales can inflate bandwidth for most users. Consider lazy-loading on first switch, or gate background loads by a feature flag.
packages/i18n/src/locales/index.ts (2)
8-105: Add literal typing for locales to improve downstream inference.Mark as const to preserve literal keys and help callers index without extra assertions.
-export const locales = { +export const locales = { en: { core: () => import("./en/core.json"), translations: () => import("./en/translations.json"), accessibility: () => import("./en/accessibility.json"), editor: () => import("./en/editor.json"), }, ... -}; +} as const;Optionally export a Loader type for stronger contracts:
export type JSONModule = { default: Record<string, unknown> }; export type LocaleLoaders = { core?: () => Promise<JSONModule>; translations?: () => Promise<JSONModule>; accessibility?: () => Promise<JSONModule>; editor?: () => Promise<JSONModule>; };
65-69: Consistent “ua” usage found; consider switching to ISO code “uk”
Constants (packages/i18n/src/constants/language.ts:17) and types (packages/i18n/src/types/language.ts:13) both use"ua"for Ukrainian, but the official ISO 639-1 code is"uk". Rename keys and values if external libraries or standards expect"uk".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/i18n/package.json(2 hunks)packages/i18n/src/index.ts(1 hunks)packages/i18n/src/locales/index.ts(1 hunks)packages/i18n/src/store/index.ts(3 hunks)packages/i18n/tsdown.config.ts(1 hunks)packages/propel/package.json(3 hunks)packages/propel/tsdown.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
PR: makeplane/plane#7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
packages/i18n/package.json
🧬 Code graph analysis (1)
packages/i18n/src/store/index.ts (1)
packages/i18n/src/locales/index.ts (1)
locales(8-105)
🪛 GitHub Check: Build and lint web apps
packages/i18n/src/store/index.ts
[warning] 158-158:
Unexpected any. Specify a different type
[warning] 158-158:
Unexpected any. Specify a different type
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (12)
packages/propel/package.json (2)
41-41: Adding cmdk dependency: LGTMThis aligns with the command components exposure.
57-59: Toolchain versions are unified
Every package declares typescript@5.8.3 and tsdown@^0.14.2 (where applicable); no inconsistencies detected.packages/i18n/package.json (3)
7-9: Dist entry points: ensure DTS is generatedThese fields are correct for dist publishing, but they rely on
dist/index.d.ts. Please enabledts: truein tsdown (see tsdown.config.ts comment) or types will be missing.
11-12: Build pipeline order is fineType-check first, then bundle with tsdown. Once DTS is enabled in tsdown, this is good.
34-36: No changes required: TypeScript and tsdown versions are consistent across all workspaces.packages/i18n/src/index.ts (1)
5-6: No name collisions detected betweenstoreandlocalesexports –storeexportsTranslationStoreandlocalesexportslocales, so re-exports won’t collide.packages/i18n/src/store/index.ts (4)
224-245: LGTM on translation fallback and caching.Formatter caching, fallback to default language, and safe key fallback are correct.
61-101: Minor: initialization flags sequencing is sane.Marking isInitialized before background loads is acceptable given core is present; loading state transitions are handled in catch paths.
8-8: Confirm dynamic JSON imports post-build
tsconfig.json in packages/i18n has"resolveJsonModule": true, and all JSON imports in src/locales/index.ts are viaimport("./…\.json")with no local tsup config overriding defaults. Build this package and smoke-test that the locale JSON files load correctly at runtime.
171-173: No numeric enum issue in ETranslationFiles
ETranslationFiles is a string enum (each member maps to a string literal), so Object.values(ETranslationFiles) returns only the intended file-name strings. No guard or changes needed.Likely an incorrect or invalid review comment.
packages/i18n/src/locales/index.ts (2)
1-5: LGTM on re-exports.Static English exports keep core available eagerly; matches store usage.
8-105: Verify dynamic JSON imports
No tsup or TypeScript config was found in the repo; confirm your bundler handles these dynamic JSON imports correctly or add an explicit JSON loader/plugin, or switch to static imports with code-splitting via intermediary TS modules.
aaryan610
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/services/package.json (2)
6-11: Dist entry fields added: looks good and resolves prior concernPointing main/module/types to dist and restricting published files fixes the earlier “source-first” issue.
30-30: Version not published on npm: pin to a released tsdown^0.14.2 isn’t available on npm at the moment; latest is 0.12.x. Recommend pinning to a published tag (e.g., ^0.12.8) and updating the lockfile.
- "tsdown": "^0.14.2", + "tsdown": "^0.12.8",Confirm the currently published version before merging. (npmjs.com)
🧹 Nitpick comments (1)
packages/services/package.json (1)
6-11: Add an exports map for robust Node/bundler resolution (optional)Consider defining conditional exports to formalize import/require/types entry points.
"types": "./dist/index.d.ts", "files": [ "dist/**" ], + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.mjs", + "require": "./dist/index.js", + "default": "./dist/index.mjs" + } + },You can also let tsdown generate these via the --exports flag if preferred. (tsdown.dev)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/services/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
PR: makeplane/plane#7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
packages/services/package.json
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/i18n/package.json (1)
21-27: Make React a peerDependency to avoid duplicate React instances and hook/runtime errors.Libraries should not ship their own React. Move react to peerDependencies (and keep as devDependency for local dev).
"dependencies": { "@plane/utils": "workspace:*", "intl-messageformat": "^10.7.11", "mobx": "^6.13.5", "mobx-react": "^9.1.0", - "lodash": "^4.17.21", - "react": "^18.3.1" + "lodash": "^4.17.21" }, + "peerDependencies": { + "react": ">=18" + }, + "peerDependenciesMeta": { + "react": { "optional": false } + }, "devDependencies": { "@plane/eslint-config": "workspace:*", "@plane/typescript-config": "workspace:*", "@types/node": "^22.5.4", "@types/lodash": "4.17.20", "@types/react": "catalog:", + "react": "catalog:", "tsdown": "catalog:", "typescript": "catalog:" }If this package also renders React components via mobx-react, consider promoting mobx and mobx-react to peerDependencies as well to avoid multiple copies at runtime.
🧹 Nitpick comments (3)
packages/i18n/package.json (2)
25-25: Consider lodash-es or per-method imports to keep bundles lean.Switch to lodash-es or import specific functions (e.g., lodash/isEmpty) to improve tree-shaking with tsdown.
18-18: Make clean script cross-platform.rm -rf is fine in CI, but rimraf is safer across OSes and paths.
- "clean": "rm -rf .turbo && rm -rf .next && rm -rf node_modules && rm -rf dist" + "clean": "rimraf .turbo .next node_modules dist"packages/logger/package.json (1)
21-22: Build/dev migration to tsdown looks correct; confirm d.ts and add optional checks.
- With "types" present, tsdown will auto-generate .d.ts; the separate "tsc --noEmit" type-check step is fine. (tsdown.dev)
- Optional: enable declaration maps for better editor nav (either tsconfig declarationMap or tsdown dts.sourcemap). (tsdown.dev)
- Optional: add a package validation script using tsdown’s built-in publint. (tsdown.dev)
Example additions outside this hunk:
{ "scripts": { "dev:types": "tsc --noEmit --watch", "check:package": "tsdown --publint" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/live/package.json(2 hunks)packages/constants/package.json(2 hunks)packages/decorators/package.json(2 hunks)packages/editor/package.json(2 hunks)packages/hooks/package.json(2 hunks)packages/i18n/package.json(2 hunks)packages/logger/package.json(2 hunks)packages/propel/package.json(3 hunks)packages/services/package.json(2 hunks)packages/types/package.json(2 hunks)packages/ui/package.json(2 hunks)packages/utils/package.json(2 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/propel/package.json
- packages/constants/package.json
- packages/types/package.json
- packages/ui/package.json
- packages/utils/package.json
- apps/live/package.json
- packages/editor/package.json
- packages/decorators/package.json
- packages/hooks/package.json
- packages/services/package.json
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/i18n/package.json (1)
11-13: DTS emission is already enabled in tsdown.config.ts (line 7). Config includesdts: true, sodist/index.d.tswill be emitted and thetypespath is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/propel/package.json (1)
7-8: Avoid double type-checking in build; DRY with existing script.You already have "check:types". Consider reusing it or relying on tsdown’s typecheck to save CI time.
- "build": "tsc --noEmit && tsdown", + "build": "pnpm run check:types && tsdown",Run to confirm tsdown config exists and that exports won’t point at TS sources at runtime:
#!/bin/bash # 1) Ensure a tsdown config is present for this package fd -a -H 'tsdown.config.*' packages/propel || echo "Missing tsdown.config.* in packages/propel" # 2) Check if exports map to src (TS) instead of built output jq -r '.exports|to_entries[]|.value' packages/propel/package.json | rg -n "^\./src/" # If this prints lines, confirm tsdown outputs per-file artifacts matching those paths # or update exports to dist equivalents.packages/i18n/package.json (1)
7-9: Add exports map to avoid dual-package hazard and ensure correct ESM/CJS resolutionWithout "exports", resolution differs across Node/bundlers. Define conditional exports aligned to your built entries.
"main": "./dist/index.js", "module": "./dist/index.mjs", "types": "./dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.mjs", + "require": "./dist/index.js", + "default": "./dist/index.mjs" + } + },
🧹 Nitpick comments (2)
packages/propel/package.json (1)
7-8: Watch flags: ensure sourcemaps/types as needed.If your tsdown config doesn’t emit sourcemaps or d.ts by default in watch, add flags/config so Storybook/consumers get good DX.
Would you like me to propose a minimal tsdown.config.ts that emits ESM+CJS+d.ts with sourcemaps?
packages/i18n/package.json (1)
7-9: Enable tree‑shaking and reduce package surface (optional)Mark as side‑effect free and restrict published files to dist.
"types": "./dist/index.d.ts", + "sideEffects": false, + "files": ["dist"],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/live/package.json(2 hunks)packages/editor/package.json(2 hunks)packages/i18n/package.json(2 hunks)packages/propel/package.json(2 hunks)packages/services/package.json(2 hunks)packages/ui/package.json(2 hunks)packages/utils/package.json(2 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- pnpm-workspace.yaml
- packages/utils/package.json
- packages/services/package.json
- packages/editor/package.json
- apps/live/package.json
- packages/ui/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
PR: makeplane/plane#7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
packages/i18n/package.json
⏰ 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: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (2)
packages/propel/package.json (1)
62-62: tsdown devDependency addition looks good.Matches the workspace migration and keeps runtime deps clean.
packages/i18n/package.json (1)
11-13: Confirm declaration file generationtsdown.config.ts already sets
dts: true, so runningtsdownshould emit.d.tsfiles underdist; please verify in CI thatdist/index.d.tsis produced.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/utils/package.json (1)
1-46: Add engines.node >=20.19 and pin CI to Node 20.19tsdown@0.14.2 requires node >=20.19.0.
Apply near the "files" field in packages/utils/package.json:
"files": [ "dist" ], + "engines": { + "node": ">=20.19" + },Pin actions/setup-node to 20.19 in workflows: pull-request-build-lint-web-apps.yml, check-version.yml.
♻️ Duplicate comments (3)
packages/i18n/package.json (2)
7-9: Add an explicit exports map to avoid dual-package hazards.Define conditional exports so Node/bundlers resolve ESM/CJS and types consistently.
"main": "./dist/index.js", "module": "./dist/index.mjs", "types": "./dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.mjs", + "require": "./dist/index.js", + "default": "./dist/index.mjs" + } + },
21-27: Move React/MobX to peerDependencies to prevent duplicate framework instances.Avoid multiple React/MobX copies across apps; keep lodash as a regular dep.
Deps block:
"dependencies": { "@plane/utils": "workspace:*", "intl-messageformat": "^10.7.11", - "mobx": "catalog:", - "mobx-react": "catalog:", "lodash": "catalog:", - "react": "catalog:" + "react": "catalog:" },Add peer deps (new section just below dependencies):
+ "peerDependencies": { + "react": "catalog:", + "mobx": "catalog:", + "mobx-react": "catalog:" + },Also add them for local dev (see next comment).
packages/utils/package.json (1)
43-45: Catalog alias: confirm tsdown resolves to a published version (avoid ^0.14.2)Past review flagged an unpublished tag. If your workspace catalog maps tsdown to an unpublished range, pin it to a published version (e.g., ^0.13.x) in the catalog.
Discover the catalog mapping:
#!/bin/bash # Show where "catalog" is defined and what tsdown maps to rg -nC2 '"catalog"\s*:' package.json pnpm-workspace.yaml -g '!**/node_modules/**' rg -nC2 '"tsdown"\s*:\s*' package.json pnpm-workspace.yaml -g '!**/node_modules/**' jq -r '.catalog.tsdown // .pnpm?.catalog?.tsdown // empty' package.json 2>/dev/null
🧹 Nitpick comments (4)
packages/ui/package.json (1)
79-79: Dev dep looks good; consider adding publish-time checks.Optionally wire tsdown’s publint/arethetypeswrong to a CI script (e.g., “check:pkg”) to catch export/type issues pre-publish. (tsdown.dev)
Example scripts (outside this hunk):
"scripts": { "check:types": "tsc --noEmit", + "check:pkg": "tsdown --publint" }packages/i18n/package.json (2)
28-36: Add React/MobX to devDependencies for local builds/tests.Keeps package buildable in isolation while staying peers at runtime.
"devDependencies": { "@plane/eslint-config": "workspace:*", "@plane/typescript-config": "workspace:*", "@types/node": "^22.5.4", "@types/lodash": "catalog:", "@types/react": "catalog:", + "react": "catalog:", + "mobx": "catalog:", + "mobx-react": "catalog:", "tsdown": "catalog:", "typescript": "catalog:" }
18-18: Tighten the clean script (nit).This lib package doesn’t produce .next/.turbo; avoid deleting app artifacts.
- "clean": "rm -rf .turbo && rm -rf .next && rm -rf node_modules && rm -rf dist" + "clean": "rm -rf dist node_modules"packages/utils/package.json (1)
7-9: Optional: add exports and sideEffects for better Node/Esm interop and tree-shakingKeeps consumers on the intended entrypoints and improves bundling.
"types": "./dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.mjs", + "require": "./dist/index.js" + } + }, + "sideEffects": false,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
apps/live/package.json(2 hunks)packages/constants/package.json(2 hunks)packages/decorators/package.json(2 hunks)packages/editor/package.json(2 hunks)packages/hooks/package.json(2 hunks)packages/i18n/package.json(2 hunks)packages/logger/package.json(2 hunks)packages/propel/package.json(2 hunks)packages/services/package.json(2 hunks)packages/types/package.json(2 hunks)packages/ui/package.json(2 hunks)packages/utils/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/constants/package.json
- packages/propel/package.json
- packages/hooks/package.json
- packages/logger/package.json
- packages/services/package.json
- packages/decorators/package.json
- apps/live/package.json
- packages/editor/package.json
- packages/types/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
PR: makeplane/plane#7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
packages/i18n/package.json
⏰ 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 (2)
packages/i18n/package.json (2)
11-13: Build scripts migration looks good.tsdown usage for dev/build is consistent with the PR goal.
28-36: No action needed: Node types align with engine constraint. The root package.json specifies"node": ">=22.18.0", which matches@types/node@^22.5.4; no updates required.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/hooks/tsdown.config.ts (1)
9-10: DTS, sourcemaps, and clean builds — niceMatches prior feedback and improves DX and publish hygiene.
🧹 Nitpick comments (1)
packages/hooks/tsdown.config.ts (1)
7-7: Also externalize JSX runtime and react-dom subpathsPrevents bundling the JSX runtime and client entry in consumers using the new JSX transform or createRoot.
Apply:
- external: ["react", "react-dom"], + external: [ + "react", + "react-dom", + "react/jsx-runtime", + "react/jsx-dev-runtime", + "react-dom/client" + ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/hooks/tsdown.config.ts(1 hunks)packages/logger/tsdown.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/logger/tsdown.config.ts
⏰ 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). (1)
- GitHub Check: Build and lint web apps
🔇 Additional comments (1)
packages/hooks/tsdown.config.ts (1)
1-1: LGTM: switched to tsdown config correctlyImport and defineConfig usage look good for the migration.
Description
Type of Change
Summary by CodeRabbit