unlock git submodule #291
unlock git submodule #291
Conversation
nossila
requested review from
anicioalexandre and
priscilladeroode
as code owners
|
WalkthroughDependency management shifted to pnpm workspace catalogs across multiple packages with version bumps and dev tooling additions. Introduces a large Wagtail GraphQL schema and centralizes Relay config. Refines test utilities with URL normalization and env var guard. Adds a TS ignore in Scrollbar and switches getTokenSSR to dynamically import next/headers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test Code
participant M as mockFetch (utils)
participant C as Config
Note over T,M: Request interception in tests
T->>M: mockFetch(url, { method, body })
M->>M: Normalize request URL (strip protocol/host, /vN, ensure leading /)
M->>M: Normalize expected URL (ensure leading /)
M->>M: Compare normalized URL + method
alt Match
M-->>T: Resolve with configured response (200-style)
else No match
M-->>T: Return 404 response
end
sequenceDiagram
autonumber
participant S as Server-side code
participant U as getTokenSSR
participant N as next/headers
S->>U: getTokenSSR()
U->>N: await import('next/headers')
U->>N: cookies()
N-->>U: CookieStore
U->>U: Read token key from cookies
U-->>S: Return token (or null)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/design-system/components/web/scrollbars/Scrollbar/index.tsx (1)
3-3: Remove ts-ignore by bridging the ref; ensuresxactually applies on desktop (non-SSR, non-mobile)
- The ignore on Line 37 can be eliminated by using a local ref and
useImperativeHandle.sxpassed toStyledScrollbarwon’t be applied because it’s a styled(SimpleBar) component, not a system-aware MUI component. Wrap the desktop branch with a Box that consumessx.Apply this diff:
-import { forwardRef, memo } from 'react' +import { forwardRef, memo, useImperativeHandle, useRef } from 'react' @@ -const Scrollbar = forwardRef<HTMLDivElement, ScrollbarProps>(({ children, sx, ...other }, ref) => { +const Scrollbar = forwardRef<HTMLDivElement, ScrollbarProps>(({ children, sx, ...other }, ref) => { const { isSSR } = useSSR() const userAgent = typeof navigator === 'undefined' ? 'SSR' : navigator.userAgent const mobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(userAgent) + const scrollRef = useRef<HTMLDivElement | null>(null) + useImperativeHandle(ref, () => scrollRef.current as HTMLDivElement | null) @@ - return ( - <Box ref={ref} sx={{ overflow: 'hidden', ...sx }} {...other}> + return ( + <Box ref={scrollRef} sx={{ overflow: 'hidden', ...sx }} {...other}> {children} </Box> ) @@ - return ( - <Box ref={ref} sx={{ overflow: 'auto', ...sx }} {...other}> + return ( + <Box ref={scrollRef} sx={{ overflow: 'auto', ...sx }} {...other}> {children} </Box> ) @@ - return ( - <StyledRootScrollbar> - <StyledScrollbar - // @ts-ignore TODO: check typing - scrollableNodeProps={{ - ref, - }} - clickOnTrack={false} - sx={sx} - {...other} - > - {children} - </StyledScrollbar> - </StyledRootScrollbar> - ) + return ( + <Box sx={sx}> + <StyledRootScrollbar> + <StyledScrollbar + scrollableNodeProps={{ ref: scrollRef }} + clickOnTrack={false} + {...other} + > + {children} + </StyledScrollbar> + </StyledRootScrollbar> + </Box> + )Also applies to: 12-16, 34-47, 37-41
packages/test/package.json (1)
11-11: Typo in lint script: use --noEmit (camelCase) for tscCurrent flag likely isn’t recognized; tsc expects --noEmit.
Apply:
- "lint": "eslint . --ext .tsx --ext .ts && tsc --noemit", + "lint": "eslint . --ext .tsx --ext .ts && tsc --noEmit",packages/design-system/package.json (1)
91-116: Catalog migration: fix unaliasedcatalog:entries in packages/design-system/package.jsonSeveral dependencies use plain
catalog:(no alias); pnpm requirescatalog:<name>and these will fail to resolve.
- Unaliased deps: jotai, js-cookie, lodash, next, react-hook-form, react-international-phone, simplebar-react, zustand.
- pnpm-workspace.yaml currently shows only
react19undercatalogs:; add the missing aliases or update the package.json entries to point to existing catalog aliases.
Action: update packages/design-system/package.json (lines 91–116) to usecatalog:<alias>for each dependency or add the corresponding catalog mappings in pnpm-workspace.yaml.
🧹 Nitpick comments (17)
packages/utils/functions/token/getTokenSSR/index.ts (2)
5-5: Remove unnecessary await on cookies().cookies() is synchronous in Next; awaiting it is redundant.
- const cookieStore = await cookies() + const cookieStore = cookies()
3-8: Add explicit types (and optionally enforce server-only usage).Improves API clarity and compile-time checks. Consider server-only guard if this package is Next-only.
-export const getTokenSSR = async (key = ACCESS_KEY_NAME) => { +export const getTokenSSR = async (key: string = ACCESS_KEY_NAME): Promise<string | undefined> => {Optional (only if all consumers run on Next.js server):
+import 'server-only'Verify consumers before adding the guard.
packages/design-system/components/web/scrollbars/Scrollbar/styled.ts (1)
6-8: Stop ignoring types; remove the ts-ignore and type the styled componentOnce the ambient module exists, this ignore should go.
Apply this diff:
-// @ts-ignore TODO: check typing -import SimpleBar from 'simplebar-react' +import SimpleBar from 'simplebar-react'Additionally, avoid casting to FC (it erases useful props like the
asprop). Prefer a typed styled component and filtersxso it doesn't leak to the DOM:// Replace the FC cast with a typed styled component export const StyledScrollbar = styled(SimpleBar, { shouldForwardProp: (prop) => prop !== 'sx', })<ScrollbarProps>(({ theme }) => ({ maxHeight: '100%', '& .simplebar-scrollbar': { '&:before': { backgroundColor: alpha(theme.palette.grey[600], 0.48) }, '&.simplebar-visible:before': { opacity: 1 }, }, '& .simplebar-mask': { zIndex: 'inherit' }, }));packages/design-system/components/web/scrollbars/Scrollbar/index.tsx (1)
20-31: Filter non-DOM props when rendering Box on SSR/mobileSpreading
...otherinto Box can leak SimpleBar-only props (e.g., clickOnTrack) into the DOM, causing React warnings.Apply this small guard:
-const Scrollbar = forwardRef<HTMLDivElement, ScrollbarProps>(({ children, sx, ...other }, ref) => { +const Scrollbar = forwardRef<HTMLDivElement, ScrollbarProps>(({ children, sx, ...other }, ref) => { const { isSSR } = useSSR() @@ const mobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(userAgent) + const { clickOnTrack, scrollableNodeProps, ...restDomProps } = other @@ - <Box ref={scrollRef} sx={{ overflow: 'hidden', ...sx }} {...other}> + <Box ref={scrollRef} sx={{ overflow: 'hidden', ...sx }} {...restDomProps}> @@ - <Box ref={scrollRef} sx={{ overflow: 'auto', ...sx }} {...other}> + <Box ref={scrollRef} sx={{ overflow: 'auto', ...sx }} {...restDomProps}>packages/test/__mocks__/fetch.ts (1)
1-5: Remove process.env mutation from shared mockpackages/test/mocks/fetch.ts is loaded via setupFilesAfterEnv (packages/test/jest.config.ts); assigning NEXT_PUBLIC_API_BASE_URL at module load mutates global state across tests. Remove the assignment from the mock and either set the default in a dedicated Jest setup file (e.g. packages/test/jest.setup.ts — process.env.NEXT_PUBLIC_API_BASE_URL ??= '' added to setupFiles/setupFilesAfterEnv) or update the mockFetch normalizers to handle undefined.
packages/test/utils/mockFetchError/index.ts (2)
34-38: Normalize expected URL consistently (version prefix, trailing slash).Avoid mismatches when tests pass
/v1/fooor trailing slashes.Apply:
- let normalizedExpectedUrl = url + let normalizedExpectedUrl = url if (!normalizedExpectedUrl.startsWith('/')) { normalizedExpectedUrl = `/${normalizedExpectedUrl}` } + normalizedExpectedUrl = normalizedExpectedUrl.replace(/^\/v\d+/, '') + if (normalizedExpectedUrl.length > 1 && normalizedExpectedUrl.endsWith('/')) { + normalizedExpectedUrl = normalizedExpectedUrl.slice(0, -1) + }
53-53: Return type inconsistency with mockFetch (Response vs plain object).mockFetch returns a POJO; here the fallback returns a
Response. Mixing shapes can make tests brittle.Consider returning a POJO like in mockFetch for the non‑match branch (or migrate both to Response for consistency).
packages/test/utils/mockFetch/index.ts (3)
18-24: Normalize path robustly and ignore query/hash.Mirror the recommended parsing from mockFetchError to avoid mismatches when
requestUrlhas a query or hash.Apply:
- // Remove protocol and domain if present - if (normalizedRequestUrl.includes('://')) { - const urlParts = normalizedRequestUrl.split('/') - const pathStartIndex = urlParts.findIndex((_, index) => index > 2) - normalizedRequestUrl = `/${urlParts.slice(pathStartIndex).join('/')}` - } + // Remove protocol/host if present using URL API when possible + if (normalizedRequestUrl.includes('://')) { + try { + normalizedRequestUrl = new URL(normalizedRequestUrl).pathname + } catch { + const parts = normalizedRequestUrl.split('/') + normalizedRequestUrl = `/${parts.slice(3).join('/')}` + } + } + + // Drop query and hash + normalizedRequestUrl = normalizedRequestUrl.split('?')[0].split('#')[0] @@ - // Ensure path starts with / + // Ensure path starts with / and normalize trailing slash (except root) if (!normalizedRequestUrl.startsWith('/')) { normalizedRequestUrl = `/${normalizedRequestUrl}` } + if (normalizedRequestUrl.length > 1 && normalizedRequestUrl.endsWith('/')) { + normalizedRequestUrl = normalizedRequestUrl.slice(0, -1) + }Also applies to: 25-33
33-38: Normalize expected URL similarly (version, trailing slash).Keeps both sides symmetric.
Apply:
- let normalizedExpectedUrl = url + let normalizedExpectedUrl = url if (!normalizedExpectedUrl.startsWith('/')) { normalizedExpectedUrl = `/${normalizedExpectedUrl}` } + normalizedExpectedUrl = normalizedExpectedUrl.replace(/^\/v\d+/, '') + if (normalizedExpectedUrl.length > 1 && normalizedExpectedUrl.endsWith('/')) { + normalizedExpectedUrl = normalizedExpectedUrl.slice(0, -1) + }
6-11: DRY: extract a shared normalizer to avoid drift between mocks.Both files duplicate this logic. Consider a tiny helper (e.g.,
packages/test/utils/url.ts) and unit tests for it.pnpm-workspace.yaml (1)
13-14: Parcel toolchain added to catalog (packager/transformer).Confirm compatibility with
parcelpinned under catalogs.eslint-plugin (2.12.0). If possible, align all Parcel bits to the same major/minor to reduce duplication.packages/design-system/package.json (1)
5-5: CSS exports can be tree‑shaken away with sideEffects: false.You export CSS files under
exports, butsideEffectsisfalse. Many bundlers will drop CSS imports unless marked as having side effects.
- Action: declare CSS as side effects.
Apply:
- "sideEffects": false, + "sideEffects": [ + "./styles/web/tailwind/globals.css", + "./styles/web/material/globals.css" + ],Also applies to: 52-57
packages/wagtail/schema.graphql (5)
277-296: Mixed pagination styles and loosely typed filters.You accept both cursor args (
before/after/first/last) and offset simultaneously, andorderBy/profileIdareString. This complicates clients and backend logic, and invites injection bugs server‑side.
- Action: pick cursor-based pagination (Relay) and drop
offset. ReplaceorderBy: Stringwith an enum of allowed fields/directions. UseprofileId: IDinstead ofString.
339-341: Typo and type improvement for comment language.
- Fix typo: “languaged used…” -> “language used…”
- Prefer
Languagesenum over rawStringforlanguage.
1797-1802: Invalid/pointless default value on enum input field.
roleType: ProfileRoles = nullsets anulldefault for an enum. Many GraphQL toolchains reject or ignore= nullon input fields; omission already implies null.
- Action: remove the default or make the field nullable without a default.
- roleType: ProfileRoles = null + roleType: ProfileRoles
1137-1143: Prefer ID types for identifiers in inputs.
PageCreateInput.user: Stringlikely represents a user identifier. UseID(and consider naming ituserId) for consistency and to enable better tooling/codegen.- user: String + userId: ID
1233-1234: Count field typed as JSON string.
commentsCount: JSONString!should be anInt(or an object with typed counters). Serializing counts as JSON harms type safety.- commentsCount: JSONString! + commentsCount: CommentsCount!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/wagtail/__generated__/.keepis excluded by!**/__generated__/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
package.json(1 hunks)packages/components/package.json(3 hunks)packages/design-system/components/web/scrollbars/Scrollbar/index.tsx(1 hunks)packages/design-system/components/web/scrollbars/Scrollbar/styled.ts(1 hunks)packages/design-system/components/web/scrollbars/Scrollbar/types.ts(1 hunks)packages/design-system/package.json(4 hunks)packages/eslint-plugin/package.json(1 hunks)packages/graphql/package.json(1 hunks)packages/provider/package.json(1 hunks)packages/test/__mocks__/fetch.ts(1 hunks)packages/test/package.json(2 hunks)packages/test/utils/mockFetch/index.ts(1 hunks)packages/test/utils/mockFetchError/index.ts(1 hunks)packages/utils/functions/token/getTokenSSR/index.ts(1 hunks)packages/utils/package.json(1 hunks)packages/wagtail/package.json(2 hunks)packages/wagtail/relay.config.js(1 hunks)packages/wagtail/schema.graphql(1 hunks)pnpm-workspace.yaml(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-23T20:19:26.771Z
Learnt from: davidbermudez-tsl
PR: silverlogic/baseapp-frontend#255
File: packages/components/modules/content-feed/web/PostItemImages/index.tsx:20-21
Timestamp: 2025-04-23T20:19:26.771Z
Learning: In the ContentFeed module, `ts-ignore` is necessary when working with RMCarousel from the react-multi-carousel library due to type definition bugs that cause build errors.
Applied to files:
packages/design-system/components/web/scrollbars/Scrollbar/index.tsxpackages/design-system/components/web/scrollbars/Scrollbar/styled.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: Analyze (javascript)
🔇 Additional comments (21)
packages/utils/functions/token/getTokenSSR/index.ts (1)
4-4: LGTM — lazy-load next/headers; confirm no client imports of getTokenSSRDynamic import is fine; sandbox search failed (rg file-type error) so I couldn't locate import sites. Run locally to confirm no client components import this util:
rg -n --glob '!node_modules' '\bgetTokenSSR\b' -g '**/*.{ts,tsx}' -S rg -l --glob '!node_modules' '\bgetTokenSSR\b' -g '**/*.{ts,tsx}' -S | xargs -r -n1 rg -nH "^\s*['\"]use client['\"]"packages/design-system/components/web/scrollbars/Scrollbar/types.ts (1)
2-3: ```shell
#!/bin/bash
set -euo pipefailecho "PWD: $(pwd)"
TARGET="packages/design-system/components/web/scrollbars/Scrollbar/types.ts"
echo "Checking target file: $TARGET"
if [ -f "$TARGET" ]; then
echo "---- BEGIN $TARGET ----"
sed -n '1,200p' "$TARGET" || true
echo "---- END $TARGET ----"
else
echo "MISSING: $TARGET"
fiecho
echo "Listing Scrollbar directory:"
ls -la packages/design-system/components/web/scrollbars/Scrollbar || trueecho
echo "Searching repository for: simplebar-react, scrollableNodeProps, @ts-ignore"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore-vcs "simplebar-react|scrollableNodeProps|@ts-ignore" || true
else
grep -RIn --line-number -e "simplebar-react" -e "scrollableNodeProps" -e "@ts-ignore" . || true
fiecho
echo "Searching for existing declaration files for simplebar-react"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore-vcs "declare module 'simplebar-react'|simplebar-react.d.ts" || true
else
grep -RIn --line-number -e "declare module 'simplebar-react'" -e "simplebar-react.d.ts" . || true
fiecho
echo "Searching package.json files for typescript dependency/version"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore-vcs '"typescript"' || true
else
grep -RIn --line-number -e '"typescript"' . || true
fiecho
echo "If present, show root and packages/design-system package.json & tsconfig.json (first 200 lines)"
for f in package.json packages/design-system/package.json tsconfig.json packages/design-system/tsconfig.json; do
if [ -f "$f" ]; then
echo "---- $f ----"
sed -n '1,200p' "$f" || true
fi
done</blockquote></details> <details> <summary>packages/test/package.json (2)</summary><blockquote> `16-24`: **axios-mock-adapter via catalog — LGTM** Change aligns with the catalog approach. --- `28-32`: **eslint plugin added — LGTM; plugin is enabled in shared config** packages/test/.eslintrc.js requires @baseapp-frontend/config/.eslintrc.js, which declares plugins: ['@baseapp-frontend'] and includes @baseapp-frontend rules. </blockquote></details> <details> <summary>packages/eslint-plugin/package.json (1)</summary><blockquote> `21-27`: **Parcel catalog pin verified — no action required** pnpm-workspace.yaml (catalogs -> eslint-plugin): parcel = ^2.12.0 — satisfies ^2.12.x. </blockquote></details> <details> <summary>packages/utils/package.json (2)</summary><blockquote> `36-59`: **Confirm @types/humps and @types/qs versions in the utils catalog** Search of pnpm-workspace.yaml returned no matches; verify the utils catalog (catalog package.json or dependency-mapping) declares @types/humps and @types/qs versions that are compatible with the runtime packages. --- `16-32`: **Approve — utils catalog entries exist** pnpm-workspace.yaml (utils block) defines: events ^3.3.0, humps ^2.0.1, jwt-decode ^4.0.0, qs ^6.14.0, server-only ^0.0.1. </blockquote></details> <details> <summary>package.json (1)</summary><blockquote> `21-27`: **LGTM — catalog entries present in pnpm-workspace.yaml** pnpm-workspace.yaml lists @parcel/packager-ts and @parcel/transformer-typescript-types at ^2.14.1; switching to catalog is fine. </blockquote></details> <details> <summary>packages/graphql/package.json (1)</summary><blockquote> `31-35`: **Approve — ESLint plugin present and wired via shared config** Confirmed: packages/config/.eslintrc.js registers the '@baseapp-frontend' plugin and rules (e.g. '@baseapp-frontend/no-process-env-comparison'); package-level .eslintrc.js files require that shared config and multiple packages (authentication, components, config, design-system, graphql, provider, test, utils, wagtail) list @baseapp-frontend/eslint-plugin in devDependencies. </blockquote></details> <details> <summary>packages/provider/package.json (1)</summary><blockquote> `23-27`: **eslint plugin added — LGTM (confirmed)** packages/provider/.eslintrc.js requires '@baseapp-frontend/config/.eslintrc.js' and packages/config/package.json includes '@baseapp-frontend/eslint-plugin', so the plugin and its rules will be applied. </blockquote></details> <details> <summary>packages/wagtail/package.json (2)</summary><blockquote> `41-41`: **Good addition: repo ESLint plugin moved to workspace.** No issues. Verify the package uses the plugin rules (extends/plugins) so the new devDependency is actually applied. --- `68-69`: **Catalog migration for dotenv(+cli) looks correct; confirm catalog entry exists.** Mapped to catalogs.components; matches pnpm-workspace.yaml. Please run a quick install in this package to ensure the CLI (`dotenv`) resolves from the catalog. </blockquote></details> <details> <summary>packages/test/utils/mockFetchError/index.ts (1)</summary><blockquote> `7-12`: **Nice: explicit handling of odd prefixes (undefined/) and multiple URL forms.** This should reduce flakiness from env misconfigurations. </blockquote></details> <details> <summary>packages/components/package.json (3)</summary><blockquote> `76-76`: **Catalog migration looks fine.** framer-motion, numbro, slugify, use-long-press moved under catalogs.components; aligns with pnpm-workspace.yaml. Also applies to: 81-81, 89-91 --- `104-104`: **Dev tool dependencies moved to catalog:components.** Looks good; keeps versions centralized. Also applies to: 109-109, 145-146 --- `152-152`: **Verify Jotai placement (devDependency vs runtime).** rg searches in packages/components returned no imports/strings referencing "jotai"; manually confirm whether any exported runtime code imports "jotai". If exported code imports it, move it to dependencies or peerDependency; devDependency is acceptable only for Storybook/tests. File: packages/components/package.json (≈ line 152) contains "jotai": "catalog:". </blockquote></details> <details> <summary>pnpm-workspace.yaml (3)</summary><blockquote> `24-24`: **simplebar-react bump to 3.3.1.** Given recent SimpleBar ESM shifts, verify any TS ignore directives or prop changes in consumers still compile without extra shims. --- `109-109`: **axios-mock-adapter added to test catalog.** Matches test infra changes; no issues. --- `161-169`: **New catalogs (components/design-system/eslint-plugin/utils) wired — workspace check OK.** Ran the provided script; no package.json 'catalog:<group>' usages lack a corresponding catalogs entry in pnpm-workspace.yaml. </blockquote></details> <details> <summary>packages/design-system/package.json (1)</summary><blockquote> `135-136`: **ESLint plugin adoption looks good.** Nice addition. Ensure project-level ESLint config enables the plugin rulesets (e.g., `"plugins": ["@baseapp-frontend"]`, and extend any recommended configs if applicable). </blockquote></details> <details> <summary>packages/wagtail/schema.graphql (1)</summary><blockquote> `1-5`: **Confirm @specifiedBy directive compatibility.** Some servers already provide `@specifiedBy`; redefining it can cause schema build errors. Ensure your GraphQL server doesn’t auto‑register it, or guard this definition to avoid duplication. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
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 (3)
packages/utils/package.json (1)
1-73: Fix catalog: placeholders in the published package.jsonPacked tarball (baseapp-frontend-utils-4.0.2.tgz → package/package.json) contains unresolved "catalog:" specifiers (examples: axios, lodash, next, typescript, peer react, @testing-library/*). Replace these placeholders with concrete semver ranges in packages/utils/package.json or implement a publish-time rewrite that resolves "catalog:" → actual versions. Repack and verify: npm pack; tar -xOf package/package.json | rg -n 'catalog:' — expect zero matches.
Location: packages/utils/package.json (packaged as package/package.json in the tarball).
packages/components/package.json (1)
1-188: Prevent publishing 'catalog:' dependency selectorsConfirmed:
npm packproduced a tarball (baseapp-frontend-components-1.4.3.tgz) whose package/package.json still contains many "catalog:..." specifiers in dependencies, devDependencies and peerDependencies. With publishConfig.access=public this will leak into the published package and break consumers — replace all "catalog:..." entries with concrete semver ranges or implement a publish-time rewrite that resolves catalog selectors (or mark the package private) before publishing.Affected: packages/components/package.json (reproduced in package/package.json inside tarball).
packages/wagtail/package.json (1)
22-29: Fix before publish: packaged package.json still contains "catalog:" selectors and publishConfig.access="public".Confirmed: npm pack produced baseapp-frontend-wagtail-1.0.38.tgz whose package/package.json (packages/wagtail/package.json) contains many "catalog:" dependency selectors and publishConfig.access="public".
Action:
- Do not publish publicly. Ensure your release pipeline rewrites all "catalog:" selectors in the packaged package.json to concrete semver ranges before publishing.
- If rewriting is not in place, gate the publish (make the package internal/private or remove public access) until the selectors are resolved.
🧹 Nitpick comments (5)
packages/wagtail/package.json (1)
24-29: Move Storybook and Next out of runtime deps.
- @storybook/react should be a devDependency.
- next should be a peerDependency to avoid duplicating/react-locking it for consumers.
Apply this diff:
"dependencies": { "@mui/material": "catalog:material-ui", "@mui/system": "catalog:material-ui", - "@storybook/react": "catalog:storybook", "axios": "catalog:", "graphql": "catalog:graphql", - "next": "catalog:", "react-relay": "catalog:graphql" }, "peerDependencies": { + "next": "catalog:", "@baseapp-frontend/design-system": "workspace:*", "@baseapp-frontend/graphql": "workspace:*", "@baseapp-frontend/utils": "workspace:*", "react": "catalog:react19" }, "devDependencies": { + "@storybook/react": "catalog:storybook",packages/components/package.json (2)
60-94: Classify Storybook/Next correctly; avoid pulling them into consumers.
- Move @storybook/react to devDependencies.
- Make next a peerDependency (libraries shouldn’t pin app frameworks).
Apply this diff:
"dependencies": { @@ - "@storybook/react": "catalog:storybook", @@ - "next": "catalog:", + // next should be declared as a peer below @@ }, "peerDependencies": { "@baseapp-frontend/authentication": "workspace:*", "@baseapp-frontend/design-system": "workspace:*", "@baseapp-frontend/graphql": "workspace:*", "@baseapp-frontend/utils": "workspace:*", + "next": "catalog:", "react": "catalog:react19", "react-dom": "catalog:react19" }, "devDependencies": { + "@storybook/react": "catalog:storybook",
1-188: Add engines/packageManager to codify toolchain (optional).Recommend adding Node/pnpm engines to prevent mismatched installs with catalogs.
Example:
"readme": "https://github.com/silverlogic/baseapp-frontend/blob/master/packages/components/README.md", + "engines": { "node": ">=18.18", "pnpm": ">=9" }, + "packageManager": "pnpm@9"packages/utils/package.json (1)
16-32: Make Next a peer (and optional) for a utils library.Having next as a runtime dependency can cause version conflicts for consumers; declare as optional peer instead.
Apply this diff:
"dependencies": { @@ - "next": "catalog:", @@ }, "peerDependencies": { - "react": "catalog:react19" + "react": "catalog:react19", + "next": "catalog:" }, + "peerDependenciesMeta": { + "next": { "optional": true } + },packages/design-system/package.json (1)
91-117: Reclassify Storybook and Next.
- Move @storybook/react to devDependencies.
- Make next a peerDependency to avoid locking consumer framework versions.
Apply this diff:
"dependencies": { @@ - "@storybook/react": "catalog:storybook", @@ - "next": "catalog:", + // next declared as a peer below @@ }, "peerDependencies": { "@baseapp-frontend/utils": "workspace:*", + "next": "catalog:", "react": "catalog:react19", "react-dom": "catalog:react19" }, "devDependencies": { + "@storybook/react": "catalog:storybook",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/authentication/CHANGELOG.md(1 hunks)packages/authentication/package.json(1 hunks)packages/components/CHANGELOG.md(1 hunks)packages/components/package.json(4 hunks)packages/config/CHANGELOG.md(1 hunks)packages/config/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/package.json(5 hunks)packages/eslint-plugin/CHANGELOG.md(1 hunks)packages/eslint-plugin/package.json(2 hunks)packages/graphql/CHANGELOG.md(1 hunks)packages/graphql/package.json(2 hunks)packages/provider/CHANGELOG.md(1 hunks)packages/provider/package.json(2 hunks)packages/test/CHANGELOG.md(1 hunks)packages/test/package.json(3 hunks)packages/utils/CHANGELOG.md(1 hunks)packages/utils/package.json(2 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(3 hunks)
✅ Files skipped from review due to trivial changes (11)
- packages/eslint-plugin/CHANGELOG.md
- packages/config/CHANGELOG.md
- packages/provider/CHANGELOG.md
- packages/wagtail/CHANGELOG.md
- packages/authentication/package.json
- packages/test/CHANGELOG.md
- packages/utils/CHANGELOG.md
- packages/config/package.json
- packages/graphql/CHANGELOG.md
- packages/authentication/CHANGELOG.md
- packages/design-system/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/provider/package.json
- packages/graphql/package.json
- packages/eslint-plugin/package.json
- packages/test/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: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (10)
packages/components/CHANGELOG.md (1)
3-12: Changelog entry looks good.Accurately documents the catalog migration and dependent bumps for 1.4.3.
packages/wagtail/package.json (2)
41-41: ESLint plugin addition is appropriate.Brings package in line with repo-wide linting.
68-69: Catalog alignment for dotenv tooling looks fine.Matches workspace strategy; scripts reference dotenv-cli correctly.
packages/components/package.json (2)
104-110: ESLint plugin addition LGTM.Consistent with other packages.
145-147: Catalog migration for dotenv tooling LGTM.Matches workspace catalogs; scripts already use dotenv-cli.
packages/utils/package.json (2)
4-4: Version bump acknowledged.v4.0.2 aligns with changelog and catalog migration.
37-39: ESLint plugin addition LGTM.Keeps package consistent with workspace lint rules.
packages/design-system/package.json (3)
4-4: Version bump to 1.1.1 looks good.Matches related changelog entry and catalog migration.
135-136: ESLint plugin addition LGTM.Aligns with repo standards.
1-186: Guard against publishingcatalog:to npm.npm pack produced a tarball but verification aborted because
teeis unavailable in the sandbox — re-run the pack+scan withoutteeand confirm there are nocatalog:tokens in package/package.json inside the tarball.#!/bin/bash set -euo pipefail cd packages/design-system npm pack --silent >/dev/null TARBALL="$(ls -t *.tgz | head -n1)" echo "Inspecting $TARBALL" tar -xOf "$TARBALL" package/package.json | rg -n 'catalog:' || echo "no matches"No matches should remain in the tarball’s manifest.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores