feat: add pnpm support
#3334
feat: add pnpm support
#3334
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request migrates script file extensions from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
storybook-tolgee-addon/package.json (1)
39-40: Consider declaring React as a peer dependency for the addonFor a Storybook addon that’s meant to be consumed by other apps, it’s usually preferable to list
react/react-domaspeerDependencies(and keep them indevDependenciesfor local build/tests) so consumers control the actual React version and you don’t accidentally bundle your own copy.A possible shape:
"devDependencies": { @@ - "react": "^17.0.2", - "react-dom": "^17.0.2", + "react": "^17.0.2", + "react-dom": "^17.0.2", @@ - }, + }, + "peerDependencies": { + "react": ">=17", + "react-dom": ">=17" + },Feel free to tune the version ranges to match what you officially support.
Also applies to: 57-58
webapp/package.json (1)
72-74: Scripts and tooling updates look consistent; consider tightening thedevelopscript
- The
.mjs→.jschanges forschema,billing-schema, andpreparelook consistent with the rest of the repo, assuminggenerate-schemas.jsandupdateBranchInfo.jsare present inwebapp/scripts/and still run correctly under"type": "module".- Calling the root
init-huskyfrom here vianpm run --prefix .. init-huskyis a reasonable way to centralize hook setup, given husky lives inwebapp.Two small follow‑ups to consider:
Fix the
developscript aliasRight now it’s:
"develop": "start"which runs a bare
startshell command, notnpm run start. A safer variant is:- "develop": "start", + "develop": "npm run start",Verify the new JS entrypoints + husky wiring
After these changes, it’s worth quickly checking:
npm run schema/npm run billing-schemastill generate the expected artifacts.npm run preparecorrectly initializes git hooks (in combination with the updated rootinit-huskyscript) and runs bothupdateBranchInfo.jsandprepareEe.jswithout errors.Also applies to: 84-84, 125-132
library/src/components/languages/FlagImage/utils.ts (1)
35-35: Consider handling missing entries or wideninggetFlagPath’s return type
getFlagPathis annotated to returnstring, butflagFiles[flagName]can beundefinedat runtime if the glob map doesn’t contain the computed name (e.g. asset missing or patterns misconfigured). That risk likely existed before, but this is a good point to decide whether you want:
- A stricter type:
export const getFlagPath = (hex: string): string | undefined => { ... }, or- A runtime fallback (e.g. falling back to the white flag asset if the specific flag is missing).
This is optional, but tightening either the typing or the runtime behavior will make downstream consumers clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
library/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlstorybook-tolgee-addon/package-lock.jsonis excluded by!**/package-lock.jsonwebapp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.husky/post-checkout(1 hunks).husky/post-commit(1 hunks)library/eslint.config.js(1 hunks)library/src/components/languages/FlagImage/utils.ts(1 hunks)package.json(1 hunks)pnpm-workspace.yaml(1 hunks)storybook-tolgee-addon/eslint.config.js(1 hunks)storybook-tolgee-addon/package.json(2 hunks)webapp/package.json(3 hunks)webapp/vite.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
webapp/**/*.{ts,tsx}: Use TypeScript path aliases (tg.component/, tg.service/, tg.hooks/, tg.views/, tg.globalContext/*) instead of relative imports
After backend API changes, regenerate TypeScript types by runningnpm run schema(andnpm run billing-schemaif applicable) in the webapp directory
Use typed React Query hooks fromuseQueryApi.tsfor API communication, not raw React Query
Use Tolgee-specific hooksuseReportEventanduseReportOncefrom 'tg.hooks/useReportEvent' for business event tracking and analytics
Files:
webapp/vite.config.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: dkrizan
Repo: tolgee/tolgee-platform PR: 3216
File: webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationRecordList.tsx:8-8
Timestamp: 2025-10-15T13:15:17.009Z
Learning: In the tolgee-platform repository, when reviewing webapp code, check `webapp/package.json` for dependencies rather than the root `package.json`, as this is a monorepo structure where the webapp has its own package configuration.
📚 Learning: 2025-11-29T10:52:15.525Z
Learnt from: zomp
Repo: tolgee/tolgee-platform PR: 3327
File: library/.storybook/preview.tsx:1-30
Timestamp: 2025-11-29T10:52:15.525Z
Learning: In Storybook 10.x, the main `storybook` package provides subpath exports like `storybook/test`, `storybook/actions`, etc. The correct triple-slash types reference for Storybook test utilities is `/// <reference types="storybook/test" />`, not `storybook/test`.
Applied to files:
storybook-tolgee-addon/eslint.config.jsstorybook-tolgee-addon/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-10-15T13:15:17.009Z
Learnt from: dkrizan
Repo: tolgee/tolgee-platform PR: 3216
File: webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationRecordList.tsx:8-8
Timestamp: 2025-10-15T13:15:17.009Z
Learning: In the tolgee-platform repository, when reviewing webapp code, check `webapp/package.json` for dependencies rather than the root `package.json`, as this is a monorepo structure where the webapp has its own package configuration.
Applied to files:
pnpm-workspace.yamlwebapp/package.json
📚 Learning: 2025-11-28T15:12:44.060Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T15:12:44.060Z
Learning: Applies to webapp/**/*.{ts,tsx} : Use TypeScript path aliases (tg.component/*, tg.service/*, tg.hooks/*, tg.views/*, tg.globalContext/*) instead of relative imports
Applied to files:
library/src/components/languages/FlagImage/utils.tslibrary/eslint.config.js
📚 Learning: 2025-06-16T20:27:09.537Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/package.json:38-41
Timestamp: 2025-06-16T20:27:09.537Z
Learning: Node.js v16.9.0+ supports wildcard subpath patterns in the "exports" field of package.json. Patterns like "./components/*.ts": "./components/*.ts" are valid syntax and are the recommended approach for exposing multiple files. This replaced the deprecated folder export syntax.
Applied to files:
webapp/vite.config.ts
📚 Learning: 2025-11-28T15:12:44.060Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T15:12:44.060Z
Learning: Applies to webapp/**/*.{ts,tsx} : After backend API changes, regenerate TypeScript types by running `npm run schema` (and `npm run billing-schema` if applicable) in the webapp directory
Applied to files:
webapp/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: BT 🔎 (server-app:runStandardTests)
🔇 Additional comments (8)
webapp/vite.config.ts (1)
54-55: Allowing the webapp root for Vite dev server FS looks fineAdding
__dirnamealongside../library/srckeeps access limited to the webapp tree and library sources, which is reasonable for local dev in this monorepo setup. No further changes needed here.package.json (1)
7-7: Double‑check husky init path with the new--prefix webappsetupUsing
npm exec --prefix webapp husky .huskyis a nice way to reuse thehuskydevDependency fromwebapp, but it also changes the working directory for the command towebapp. If your.huskydirectory actually lives at the repo root (as the hooks in this PR suggest), you may need to adjust the argument (for example to../.husky) or rely on husky’s default target directory instead.Please run
npm run init-huskyonce and confirm:
- Git hooks are created/updated, and
- They point at the intended
.huskydirectory (root vswebapp/.husky).If they land in the wrong place, we should tweak the script.
.husky/post-checkout (1)
1-1: Hook update to.jsscript is consistent with script renamesSwitching the hook to
webapp/scripts/updateBranchInfo.jsmatches the rest of the.mjs→.jsmigration. Nothing else to adjust here..husky/post-commit (1)
1-1: Post‑commit hook now points to the.jsscript – looks goodThis keeps the hook in sync with the renamed
updateBranchInfo.jsscript; behavior stays the same.pnpm-workspace.yaml (1)
1-5: Workspace definition is minimal and correct; keep an eye on future packagesThe
pnpm-workspace.yamlis well‑formed and the three listed packages match the main JS workspaces in this repo. This is a good starting point.If you later add more JS packages, you may want to switch to a glob pattern (e.g.
packages: ['*/']or similar) to avoid having to maintain a manual list, but that’s purely a convenience choice.library/eslint.config.js (1)
30-36: SettingtsconfigRootDiris a solid fix for TS config resolutionAdding
tsconfigRootDir: import.meta.dirnameunderlanguageOptions.parserOptionsis the right pattern for flat configs in a workspace: it makes the TS ESLint parser resolvetsconfig.jsonrelative to thelibrarypackage instead of the current working directory, which is especially helpful with pnpm and monorepo tooling.No further adjustments needed here.
storybook-tolgee-addon/eslint.config.js (1)
29-29: Good addition for TypeScript ESLint resolution in monorepo.Setting
tsconfigRootDir: import.meta.dirnameanchors the TypeScript config resolution to this package's directory, which is a recommended practice for monorepo setups. The project's Node.js version requirement (>= 22.10.0) fully supportsimport.meta.dirname.library/src/components/languages/FlagImage/utils.ts (1)
26-26: DerivingflagNamesfrom normalizedflagFileslooks goodUsing
Object.keys(flagFiles)after normalizing keys viaremovePathsis straightforward and keepsflagNamesin sync with whatever layout actually matched.
| const flagFiles = removePaths({ | ||
| // if package-local install (npm) | ||
| ...import.meta.glob<true, string, string>( | ||
| '/node_modules/@tginternal/language-util/flags/*.svg', | ||
| { | ||
| query: '?url', | ||
| import: 'default', | ||
| // base: '/node_modules/@tginternal/language-util/', | ||
| eager: true, | ||
| }, | ||
| { query: '?url', import: 'default', eager: true }, | ||
| ), | ||
| // if hoisted one level up (pnpm) | ||
| ...import.meta.glob<true, string, string>( | ||
| '../../../../../node_modules/@tginternal/language-util/flags/*.svg', | ||
| { query: '?url', import: 'default', eager: true }, | ||
| ), | ||
| '/node_modules/@tginternal/language-util/', | ||
| ); | ||
| export const flagNames = Object.keys(flagPaths).map((path) => | ||
| path.replace(/^\.\/flags\/(.*)\.svg$/, '$1'), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Vite 7.2.x import.meta.glob generic parameters TypeScript types signature
💡 Result:
Short answer — Vite’s client types let you pass a generic to import.meta.glob to type the module shape. The commonly used TypeScript signature (as shipped in vite/client.d.ts and shown in many community examples) is:
declare global {
interface ImportMeta {
glob<T = any>(
pattern: string | string[],
options?: GlobOptions
): Record<string, () => Promise>;
globEager<T = any>(
pattern: string | string[],
options?: GlobOptions & { eager?: true }
): Record<string, T>;
env: ImportMetaEnv;
hot?: any;
}
}
- import.meta.glob(...) returns a map of lazy import functions returning Promise.
- import.meta.globEager(...) (when used / eager) returns a map of T directly.
Sources: Vite docs (Glob Import / Features) and community TypeScript examples showing the same augmentation. [1][2]
References
[1] Vite — Features / Glob Import (describes glob / globEager behavior).
[2] Community TS declaration examples (augmentation showing ImportMeta.glob used in practice).
🌐 Web query:
Vite import.meta.glob monorepo workspace node_modules patterns recommended
💡 Result:
Summary — recommended patterns & practices when using import.meta.glob in a monorepo (node_modules / workspace)
Key points
- import.meta.glob only matches files inside the resolved import specifier space (relative, absolute-from-root, or alias) and is intended for project files; treat node_modules as off-limits for broad globs. (vite.dev)
- Vite historically does not search node_modules with glob imports (you can get empty results if you try to glob /node_modules). Avoid relying on globbing into node_modules. (lightrun.com)
- In monorepos, configure Vite’s workspace/root and server.fs.allow so the dev server can access hoisted files when needed (or explicitly allow the workspace root). (v4.vite.dev)
Practical recommendations (concise)
- Prefer package-local globs (inside each package) instead of globbing the workspace root or node_modules:
- Good: import.meta.glob('./src/**/*.ts') (inside the package). (vite.dev)
- Explicitly exclude node_modules when you must use wide globs:
- Example: import.meta.glob(['.//*.js', '!/node_modules/**']). (vite.dev)
- Do NOT depend on import.meta.glob to discover files inside node_modules; instead:
- Import the package’s entry or list the specific files you need, or use the package API that exposes its resources. (If a package exposes many locale files, prefer the package’s loader or explicit imports.)
- If you need dynamic access to package files, enumerate them at build time (script) or ship a manifest in the package.
(lightrun.com)
- If your monorepo hoists node_modules (pnpm/yarn) and Vite blocks access, set server.fs.allow / searchForWorkspaceRoot to include the workspace root so Vite can serve hoisted files during dev:
- Example in vite.config.js: server.fs.allow: [ searchForWorkspaceRoot(process.cwd()), '..' ]. (v4.vite.dev)
- Be careful mixing eager and non‑eager glob imports — they can change bundling/Code‑split behavior; prefer a single approach per import set. (github.com)
- Use negative/glob arrays and alias paths (resolve.alias) for precise control; globs must be literals. (vite.dev)
If you want, I can:
- Give a concrete vite.config.js + import.meta.glob examples for your repo layout (provide repo structure), or
- Search specific package cases (e.g. dayjs locales) and show how to load those without globbing node_modules.
Citations:
- 1: https://vite.dev/guide/features.html?utm_source=openai
- 2: https://lightrun.com/answers/vitejs-vite-glob-import-doesnt-search-under-node_modules?utm_source=openai
- 3: https://v4.vite.dev/config/server-options?utm_source=openai
- 4: https://vite.dev/guide/features.html?utm_source=openai
- 5: https://vite.dev/guide/features.html?utm_source=openai
- 6: https://lightrun.com/answers/vitejs-vite-glob-import-doesnt-search-under-node_modules?utm_source=openai
- 7: https://v4.vite.dev/config/server-options?utm_source=openai
- 8: Code splitting doesn't work when combining regular glob import and eager glob import. vitejs/vite#12292
- 9: https://vite.dev/guide/features.html?utm_source=openai
🏁 Script executed:
# Search for import.meta.glob usage and type definitions in the repo
rg "import\.meta\.glob" -A 3 -B 1 --max-count 20Repository: tolgee/tolgee-platform
Length of output: 999
🏁 Script executed:
# Look for vite client type augmentations or custom type definitions
fd "\.d\.ts$|vite" -t f | head -20
rg "declare global|ImportMeta|import.meta.glob" --max-count 30Repository: tolgee/tolgee-platform
Length of output: 1107
🏁 Script executed:
# Check the actual vite version in package.json or lock files
fd "package.json|package-lock.json|pnpm-lock.yaml|yarn.lock" -t f | head -5
cat package.json 2>/dev/null | head -50Repository: tolgee/tolgee-platform
Length of output: 1433
🏁 Script executed:
cat -n library/custom.d.tsRepository: tolgee/tolgee-platform
Length of output: 608
🏁 Script executed:
cat -n library/package.json | head -80Repository: tolgee/tolgee-platform
Length of output: 3276
🏁 Script executed:
rg "glob.*<.*,.*,.*>" --max-count 10
rg "declare.*glob|type.*glob" -A 2 --max-count 20Repository: tolgee/tolgee-platform
Length of output: 21406
🏁 Script executed:
# Check if there are any tsconfig files that might give more context
fd "tsconfig" -t f
cat -n library/tsconfig.build.json 2>/dev/null | head -50Repository: tolgee/tolgee-platform
Length of output: 666
Fix the import.meta.glob generics and reconsider the node_modules globbing approach
The generic parameters <true, string, string> are incorrect for Vite 7.2.2. The standard signature supports only a single type parameter: import.meta.glob<T>(...). More importantly, Vite's documentation explicitly discourages globbing into node_modules—it's not a supported use case and the paths /node_modules/... and ../../../../../node_modules/... tightly couple this to your current repo layout.
Instead, configure Vite's server.fs.allow to grant workspace access to hoisted dependencies, or import the flags from @tginternal/language-util via its public API (e.g., a manifest or loader function the package should export). This eliminates the layout coupling and aligns with Vite's intended patterns.
JanCizmar
left a comment
JanCizmar
left a comment
There was a problem hiding this comment.
Looks like you would like to get higher in line change statistics 👍
zomp
force-pushed
the
storybook-itself
branch
from
7cd2462 to
57e38be
Compare
List of changes to be done to add pnpm support.
Reasons to use pnpm:
Reasons to stay with npm:
The PR is not complete. If we decide to go the pnpm way, we should:
package-lock.jsonfiles in favor of singlepnpm-lock.yaml.installWebappDeps...).Other notes:
npm install --prefix library && npm install --prefix webappwill be necessary. IMO pnpm will not make sense then. ... Unless we decide to move a monorepo way.nodeLinker: hoisted). We have to, because we also have to preserve symlinks (preserveSymlinks: true) in Vite because of local dependency modules loading (e.g. dependencies of /library have to be relative to library project when library is directly included from /webapp, not relative to webapp itself – e.g. some dependencies would not be present or on different versions there).Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.