Conversation
carlagn
requested review from
AmanVarshney01,
ArthurGamby,
aidankmcalister,
ankur-arch,
mhartington and
mhessdev
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Next.js careers page and multiple client-side components (Flexible SVG, StatsList, OpenRoles, Masonry wrapper), several CSS animation modules, a masonry type declaration, and a BOARD_ID constant sourced from env; page composes these pieces into a careers UI with anchored open positions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (15)
apps/site/src/components/navigation-wrapper.tsx (1)
6-20: Avoid duplicating theLinkcontract locally.This local
Linkinterface can drift from the source type in@prisma-docs/ui/components/web-navigation. Prefer importing the shared type.Proposed refactor
import { WebNavigation } from "@prisma-docs/ui/components/web-navigation"; +import type { Link } from "@prisma-docs/ui/components/web-navigation"; import { usePathname } from "next/navigation"; -interface Link { - text: string; - external?: boolean; - url?: string; - icon?: string; - desc?: string; - col?: number; - sub?: Array<{ - text: string; - external?: boolean; - url: string; - icon?: string; - desc?: string; - }>; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/navigation-wrapper.tsx` around lines 6 - 20, You defined a local Link interface in navigation-wrapper.tsx that duplicates the shared type; replace it by importing the canonical Link type from `@prisma-docs/ui/components/web-navigation` (or the exported name used there) and remove the local interface declaration so the component uses the imported Link type (update any usages in this file to reference the imported symbol).apps/site/src/components/careers/worldmap.module.css (1)
1-189: Consider using CSS custom properties for maintainability.The staggered animation pattern works well. For easier maintenance, you could extract the repeated colors into CSS custom properties at the
.worldmaplevel:.worldmap { --fill-primary: rgb(90, 103, 216); --fill-secondary: rgb(255, 255, 255); --fill-accent: rgb(163, 191, 250); }This is optional—the current implementation functions correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/worldmap.module.css` around lines 1 - 189, Extract repeated color values into CSS custom properties on the .worldmap selector (e.g., --fill-primary, --fill-secondary, --fill-accent) and replace the hardcoded rgb(...) values used in the .worldmap.active .svg-elem-2, .svg-elem-3, .svg-elem-4, .svg-elem-5, etc. selectors with those variables (var(--fill-...)); keep the transition and timing on .worldmap .svg-elem-N as-is so only the fill values are centralized for maintainability.apps/site/src/components/careers/masonry-client.tsx (1)
5-11: Consider adding a loading fallback to prevent layout shift.When the masonry component loads asynchronously, there's no placeholder content. This can cause layout shift (CLS), which affects Core Web Vitals. A skeleton or placeholder could improve perceived performance.
const MasonryPict = dynamic( () => import("./masonry").then((mod) => ({ default: mod.MasonryPict, })), - { ssr: false }, + { + ssr: false, + loading: () => <div className="min-h-[400px]" /> + }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/masonry-client.tsx` around lines 5 - 11, The dynamic import of MasonryPict currently has no loading fallback and can cause layout shift; update the dynamic call that defines MasonryPict to pass a loading option (e.g., loading: () => <Skeleton/> or a simple placeholder element) so a stable skeleton/placeholder is rendered while the async component (MasonryPict) loads, ensuring the placeholder mimics the component’s dimensions to avoid CLS.apps/site/src/components/careers/open-roles.tsx (2)
14-14: Heavy use ofanytypes reduces type safety.Multiple
anyannotations bypass TypeScript's benefits. Consider defining interfaces for the job data structure and component props for better maintainability and catching issues at compile time.interface Job { id: string; name: string; url: string; department: { label: string }; workLocation?: { label: string }; dept?: string; } interface OpenRolesMenuProps { setFilter: (value: string) => void; filters: string[]; }Also applies to: 45-52, 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/open-roles.tsx` at line 14, Replace the broad any annotations by defining concrete types and applying them to the component and related variables: create an OpenRolesMenuProps interface (with setFilter: (value: string) => void and filters: string[]) and a Job interface (id, name, url, department: { label: string }, optional workLocation, optional dept) and use these to type the OpenRolesMenu props and any job-related variables/arrays used inside map/rendering (e.g., job items, filters handling). Update other occurrences of any in the same file (around the areas mentioned, e.g., where jobs are mapped or filter state is used) to use these interfaces so setFilter, filters, and job properties are strongly typed.
86-86:Object.keys(jobs).lengthis falsy when zero—renders nothing without feedback.When there are no jobs, or while loading, the UI shows nothing. Consider adding a loading indicator and an empty state message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/open-roles.tsx` at line 86, The conditional using Object.keys(jobs).length renders nothing when jobs is empty or loading; update the OpenRoles render logic to explicitly check for three states: loading, empty, and populated—replace the truthy check Object.keys(jobs).length with a clear condition like Object.keys(jobs).length > 0 for the populated branch, add a loading indicator (spinner or "Loading..." JSX) when an isLoading prop/state is true (or while jobs is undefined/null), and render an empty-state message/component when Object.keys(jobs).length === 0; reference the jobs variable and the OpenRoles component/render function when making these changes.apps/site/src/app/global.css (1)
196-198: Global Swiper override may affect other carousels.This rule applies to all
.swiper-button-prevelements site-wide. If another Swiper instance is added elsewhere, it will inherit this positioning. Consider scoping it to the testimonials carousel specifically:.testimonials-swiper .swiper-button-prev { top: 100% !important; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/global.css` around lines 196 - 198, The global CSS rule targets all .swiper-button-prev and will affect every Swiper instance; scope this override to only the testimonials carousel by replacing the selector with a more specific one such as .testimonials-swiper .swiper-button-prev (and mirror for .swiper-button-next if needed) so the positioning change only applies to the testimonials Swiper.apps/site/src/components/careers/testimonials.tsx (2)
60-64: Usingtestimonial.bodyas key could cause issues with duplicate content.If two testimonials ever have identical body text, React will have key collisions. Consider using a more stable identifier or combining with the index.
💡 Safer key approach
- {testimonials.map((testimonial: Testimonial) => ( - <SwiperSlide key={testimonial.body} className="my-auto"> + {testimonials.map((testimonial: Testimonial, index: number) => ( + <SwiperSlide key={`${index}-${testimonial.body.slice(0, 20)}`} className="my-auto">Or better yet, if you control the data shape, add an
idfield to each testimonial.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/testimonials.tsx` around lines 60 - 64, The current map uses the testimonial.body string as the React key which can collide when bodies are identical; update the mapping in testimonials.map to use a stable unique identifier (e.g., testimonial.id) for the SwiperSlide key, falling back to a combo like `${testimonial.id || testimonial.author}-${index}` or the index only if no id exists; modify the Testimonial data shape to include an id when possible and update the SwiperSlide key prop and any related usages of Testimonial to reference that id.
2-2: Unused import:useRefis imported butReact.useRefis used instead.Line 2 imports
useRefdirectly, but line 28 usesReact.useRef. Pick one approach for consistency and remove the unused import.🧹 Clean up imports
"use client"; -import { useRef } from "react"; import { Navigation, Pagination, Scrollbar, A11y } from "swiper/modules"; import { Swiper, SwiperSlide } from "swiper/react"; import type { Swiper as SwiperType } from "swiper";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/testimonials.tsx` at line 2, The import of useRef is unused because the code uses React.useRef; fix by making the usage and import consistent—either remove the named import useRef from the import list and keep React.useRef usage, or change React.useRef to useRef and keep the named import; update the import/export line accordingly and remove the unused identifier so there are no lint warnings related to useRef (reference symbols: useRef import and React.useRef usage in this component).apps/site/src/components/careers/flexible.module.css (1)
33-36: Floating-point precision artifacts in transition delays are cosmetic noise.Values like
0.07500000000000001sare functionally identical to0.075s, but they suggest this CSS was generated programmatically. If you're maintaining this manually going forward, consider cleaning up these values for readability. If it's generated, this is fine to leave as-is./* Current */ transition: fill 0.6s cubic-bezier(0.47, 0, 0.745, 0.715) 0.07500000000000001s; /* Cleaner */ transition: fill 0.6s cubic-bezier(0.47, 0, 0.745, 0.715) 0.075s;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/flexible.module.css` around lines 33 - 36, The transition declarations in flexible.module.css contain floating-point artifact delays (e.g. 0.07500000000000001s); update both the -webkit-transition and transition declarations to use the cleaned value 0.075s instead of the long floating-point representation so the CSS reads clearly and behaves identically (look for the transition and -webkit-transition properties in flexible.module.css and replace the long decimal delay).apps/site/src/components/careers/masonry.tsx (1)
2-3: Remove the@ts-ignorecomment since types are now declared.You've added type declarations in
apps/site/src/types/react-responsive-masonry.d.ts, so the ignore directive is no longer necessary and may mask future type issues.🧹 Suggested fix
"use client"; -// `@ts-ignore` - no types available for react-responsive-masonry import Masonry, { ResponsiveMasonry } from "react-responsive-masonry";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/masonry.tsx` around lines 2 - 3, Remove the obsolete "// `@ts-ignore` - no types available for react-responsive-masonry" comment above the import of Masonry and ResponsiveMasonry in the masonry.tsx file (where you import from "react-responsive-masonry"); since you added types in apps/site/src/types/react-responsive-masonry.d.ts, delete that ts-ignore and run TypeScript typecheck to confirm no errors, and also search for any other lingering ts-ignore comments related to react-responsive-masonry and remove them.apps/site/src/app/careers/page.tsx (4)
1-1: Unused import:Antigravityis imported but never used.The
Antigravitycomponent is imported from the homepage components but doesn't appear anywhere in the JSX.🧹 Remove unused import
-import Antigravity from "../../components/homepage/antigravity";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` at line 1, Remove the unused import statement for the Antigravity component: delete the line importing Antigravity (import Antigravity from "../../components/homepage/antigravity";) from page.tsx since the Antigravity symbol is not referenced in the JSX; alternatively, if the component is intended to be used, add it into the rendered JSX where appropriate (referencing the Antigravity component name) instead of leaving the unused import.
9-9: Unused import:reviewfrom homepage.json is not used.This import doesn't appear to be referenced anywhere in the component.
🧹 Remove unused import
-import review from "../../data/homepage.json";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` at line 9, The import "review" from "../../data/homepage.json" is unused in the careers page component; remove the unused import statement (the line importing review) from apps/site/src/app/careers/page.tsx, or if the data is actually needed, reference the symbol "review" in the component (e.g., within the CareersPage function) — but prefer deleting the import to eliminate the unused symbol.
192-201: Usenext/imageinstead of<img>for automatic optimization.Next.js's
Imagecomponent provides automatic lazy loading, responsive sizing, and format optimization. Using raw<img>tags bypasses these benefits.🖼️ Suggested refactor
+import Image from "next/image"; // In the JSX: - <img + <Image src="/illustrations/careers/hero_lines.svg" alt="Hero lines" + fill className="w-full h-full object-cover object-bottom hidden dark:block" /> - <img + <Image src="/illustrations/careers/hero_lines_light.svg" alt="Hero lines" + fill className="w-full h-full object-cover object-bottom block dark:hidden" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 192 - 201, Replace the raw <img> tags with Next.js' Image component: import Image from 'next/image' at the top of the file and swap each <img src=... alt=... className=...> for <Image> using the same src and alt, but use the fill prop (or explicit width/height) to preserve the original full-bleed behavior and keep the className (e.g., "object-cover object-bottom hidden dark:block" and "object-cover object-bottom block dark:hidden"); this enables automatic optimization, lazy-loading and responsive sizing for the two hero images.
324-327:dangerouslySetInnerHTMLis low-risk here but sets a risky pattern.The static analysis tool flagged this, and while the data is hardcoded in the same file (so XSS isn't an immediate concern), this pattern could be copied elsewhere with dynamic data. Consider using a React fragment with explicit elements instead:
💡 Safer alternative
Instead of HTML strings in the
benefitsarray:{ icon: "fa-regular fa-arrow-trend-up", title: "Stock options package", description: "with a maximum exercise period of 10 years after grant", }Then in the JSX:
<p className="text-lg"> <b>{benefit.title}</b> {benefit.description} </p>This eliminates the need for
dangerouslySetInnerHTMLentirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 324 - 327, The code uses dangerouslySetInnerHTML on benefit.text which is risky and sets a pattern for future XSS; instead refactor the benefits data to store structured fields (e.g., title and description) and update the JSX that renders each benefit (the loop that references benefit and the <p> using dangerouslySetInnerHTML) to render elements directly (e.g., <b>{benefit.title}</b> and plain text for description) so you can remove dangerouslySetInnerHTML entirely and render with plain React nodes; update the benefits array entries and the renderer where benefit.text is used to reference benefit.title and benefit.description (or similar field names) and render them as normal JSX.apps/site/src/components/careers/stats-list.tsx (1)
6-11: Define a proper interface forstatsListitems instead of usingany.Using
any[]bypasses TypeScript's type checking entirely. This makes it easy to pass malformed data and harder to catch bugs at compile time. Consider defining an interface:🛠️ Suggested improvement
+"use client"; +import dynamic from "next/dynamic"; + +interface StatItem { + head: number; + sub: string; +} + const AnimatedNumbers = dynamic(() => import("react-animated-numbers"), { ssr: false, }); -export const StatsList = ({ statsList }: { statsList: any[] }) => { +export const StatsList = ({ statsList }: { statsList: StatItem[] }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/stats-list.tsx` around lines 6 - 11, The component currently accepts statsList: any[] which disables type checking; define a proper interface (e.g., interface StatItem { title: string; value: string | number; description?: string }) and change the StatsList prop type to statsList: StatItem[] and the map item type from any to StatItem, then update any property access in StatsList to use those typed fields (StatsList, statsList, StatItem are the identifiers to change) so TypeScript can validate the shape of each stat entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/careers/page.tsx`:
- Around line 182-185: The page's exported metadata uses homepage constants
(SITE_HOME_TITLE, SITE_HOME_DESCRIPTION) instead of careers-specific values;
update the metadata object exported as metadata to reference careers-specific
constants (e.g., CAREERS_TITLE and CAREERS_DESCRIPTION) or create and import
those new constants from the shared metadata/constants module, then replace
SITE_HOME_TITLE and SITE_HOME_DESCRIPTION with the new CAREERS_* constants in
the export.
In `@apps/site/src/components/careers/masonry.tsx`:
- Around line 20-28: The Image components inside the images.map in the masonry
component are using a hardcoded alt ("hello") which breaks accessibility; change
the alt prop to use the image metadata (e.g., replace alt="hello" with the
actual field from the mapped item such as e.imageAlt or e.altText) so each
<Image> renders a descriptive alt string from the images array used by the
masonry component.
In `@apps/site/src/components/careers/open-roles.tsx`:
- Line 67: The sort comparator currently uses a single-argument callback and
must be replaced with a two-argument comparator for stable ordering: update the
.sort call so the comparator accepts (a, b), computes whether a.name or b.name
includes "General Applications", returns positive if a is "General Applications"
and b is not (to push it after), negative if b is "General Applications" and a
is not (to keep normal items first), and returns 0 (or falls back to comparing
a.name and b.name) when both are the same type; modify the inline .sort((a: any)
=> ...) comparator accordingly.
- Around line 58-77: The fetchJobs function currently has no error handling—wrap
the async fetch and downstream processing in a try/catch inside fetchJobs, catch
network/JSON/parsing errors, and on error call a state updater (e.g.,
setJobs([]) and setJobsError or setJobsLoading(false)) so the component can
display an error or fallback UI; also log the caught error (console.error or
processLogger) and ensure you only call setJobs with the grouped result when the
fetch and mapping succeed (references: fetchJobs, setJobs, rdata,
rjobsWithDept).
In `@apps/site/src/components/careers/stats-list.tsx`:
- Line 22: The expression e.sub.toLowerCase() can throw if e.sub is
null/undefined; update the conditional in the stats-list rendering to guard or
normalize e.sub before calling toLowerCase (e.g., use optional chaining or
coerce to a string) so the check (previously written as e.sub.toLowerCase() ===
"remote") safely handles missing values; locate the conditional using the e.sub
symbol in the render JSX and replace it with a null-safe variant.
In `@apps/site/src/components/constants.ts`:
- Line 1: The BOARD_ID export can be undefined when NEXT_PUBLIC_BOARD_ID is not
set which leads to invalid fetch URLs (e.g., in open-roles.tsx); update the
export in constants.ts to validate at module load: check
process.env.NEXT_PUBLIC_BOARD_ID and either throw a clear error if missing or
provide a safe fallback value, then export the validated/normalized BOARD_ID so
consumers like open-roles.tsx always receive a defined string (reference symbol:
BOARD_ID).
In `@apps/site/src/components/navigation-wrapper.tsx`:
- Around line 37-41: The current exact path check in navigation-wrapper.tsx is
too strict and misses "/careers/", "/careers/anything" etc.; update the logic
that uses orm.includes(pathname.split("?")[0]) to normalize the path (strip
query and trailing slash) and perform prefix matching: e.g., compute const
pathnameNoQuery = pathname.split("?")[0].replace(/\/+$/,'') and then use
orm.some(p => pathnameNoQuery === p || pathnameNoQuery.startsWith(p + "/")) (or
specifically check pathnameNoQuery.startsWith("/careers")) so careers and nested
routes map to the "orm" variant instead of falling back to "ppg".
In `@apps/site/src/types/react-responsive-masonry.d.ts`:
- Around line 16-17: The declaration file references React.Component but never
imports React; add an explicit import of React (e.g. import * as React from
'react') at the top of the module declaration so React is in scope, then keep
the exported classes (ResponsiveMasonry and Masonry) as extending
React.Component; this ensures the symbols ResponsiveMasonry and Masonry compile
correctly in the declaration file.
In `@packages/ui/src/components/quote.tsx`:
- Around line 39-53: The Separator can render at the start when author.title is
falsy but author.company exists; update the JSX in the quote component so the
Separator is only rendered when both author.title and author.company are present
(i.e., wrap Separator in a conditional that checks both), leaving the existing
title and company spans unchanged; reference the author.title, author.company
checks and the Separator element in the component to locate and adjust the
rendering logic.
---
Nitpick comments:
In `@apps/site/src/app/careers/page.tsx`:
- Line 1: Remove the unused import statement for the Antigravity component:
delete the line importing Antigravity (import Antigravity from
"../../components/homepage/antigravity";) from page.tsx since the Antigravity
symbol is not referenced in the JSX; alternatively, if the component is intended
to be used, add it into the rendered JSX where appropriate (referencing the
Antigravity component name) instead of leaving the unused import.
- Line 9: The import "review" from "../../data/homepage.json" is unused in the
careers page component; remove the unused import statement (the line importing
review) from apps/site/src/app/careers/page.tsx, or if the data is actually
needed, reference the symbol "review" in the component (e.g., within the
CareersPage function) — but prefer deleting the import to eliminate the unused
symbol.
- Around line 192-201: Replace the raw <img> tags with Next.js' Image component:
import Image from 'next/image' at the top of the file and swap each <img src=...
alt=... className=...> for <Image> using the same src and alt, but use the fill
prop (or explicit width/height) to preserve the original full-bleed behavior and
keep the className (e.g., "object-cover object-bottom hidden dark:block" and
"object-cover object-bottom block dark:hidden"); this enables automatic
optimization, lazy-loading and responsive sizing for the two hero images.
- Around line 324-327: The code uses dangerouslySetInnerHTML on benefit.text
which is risky and sets a pattern for future XSS; instead refactor the benefits
data to store structured fields (e.g., title and description) and update the JSX
that renders each benefit (the loop that references benefit and the <p> using
dangerouslySetInnerHTML) to render elements directly (e.g.,
<b>{benefit.title}</b> and plain text for description) so you can remove
dangerouslySetInnerHTML entirely and render with plain React nodes; update the
benefits array entries and the renderer where benefit.text is used to reference
benefit.title and benefit.description (or similar field names) and render them
as normal JSX.
In `@apps/site/src/app/global.css`:
- Around line 196-198: The global CSS rule targets all .swiper-button-prev and
will affect every Swiper instance; scope this override to only the testimonials
carousel by replacing the selector with a more specific one such as
.testimonials-swiper .swiper-button-prev (and mirror for .swiper-button-next if
needed) so the positioning change only applies to the testimonials Swiper.
In `@apps/site/src/components/careers/flexible.module.css`:
- Around line 33-36: The transition declarations in flexible.module.css contain
floating-point artifact delays (e.g. 0.07500000000000001s); update both the
-webkit-transition and transition declarations to use the cleaned value 0.075s
instead of the long floating-point representation so the CSS reads clearly and
behaves identically (look for the transition and -webkit-transition properties
in flexible.module.css and replace the long decimal delay).
In `@apps/site/src/components/careers/masonry-client.tsx`:
- Around line 5-11: The dynamic import of MasonryPict currently has no loading
fallback and can cause layout shift; update the dynamic call that defines
MasonryPict to pass a loading option (e.g., loading: () => <Skeleton/> or a
simple placeholder element) so a stable skeleton/placeholder is rendered while
the async component (MasonryPict) loads, ensuring the placeholder mimics the
component’s dimensions to avoid CLS.
In `@apps/site/src/components/careers/masonry.tsx`:
- Around line 2-3: Remove the obsolete "// `@ts-ignore` - no types available for
react-responsive-masonry" comment above the import of Masonry and
ResponsiveMasonry in the masonry.tsx file (where you import from
"react-responsive-masonry"); since you added types in
apps/site/src/types/react-responsive-masonry.d.ts, delete that ts-ignore and run
TypeScript typecheck to confirm no errors, and also search for any other
lingering ts-ignore comments related to react-responsive-masonry and remove
them.
In `@apps/site/src/components/careers/open-roles.tsx`:
- Line 14: Replace the broad any annotations by defining concrete types and
applying them to the component and related variables: create an
OpenRolesMenuProps interface (with setFilter: (value: string) => void and
filters: string[]) and a Job interface (id, name, url, department: { label:
string }, optional workLocation, optional dept) and use these to type the
OpenRolesMenu props and any job-related variables/arrays used inside
map/rendering (e.g., job items, filters handling). Update other occurrences of
any in the same file (around the areas mentioned, e.g., where jobs are mapped or
filter state is used) to use these interfaces so setFilter, filters, and job
properties are strongly typed.
- Line 86: The conditional using Object.keys(jobs).length renders nothing when
jobs is empty or loading; update the OpenRoles render logic to explicitly check
for three states: loading, empty, and populated—replace the truthy check
Object.keys(jobs).length with a clear condition like Object.keys(jobs).length >
0 for the populated branch, add a loading indicator (spinner or "Loading..."
JSX) when an isLoading prop/state is true (or while jobs is undefined/null), and
render an empty-state message/component when Object.keys(jobs).length === 0;
reference the jobs variable and the OpenRoles component/render function when
making these changes.
In `@apps/site/src/components/careers/stats-list.tsx`:
- Around line 6-11: The component currently accepts statsList: any[] which
disables type checking; define a proper interface (e.g., interface StatItem {
title: string; value: string | number; description?: string }) and change the
StatsList prop type to statsList: StatItem[] and the map item type from any to
StatItem, then update any property access in StatsList to use those typed fields
(StatsList, statsList, StatItem are the identifiers to change) so TypeScript can
validate the shape of each stat entry.
In `@apps/site/src/components/careers/testimonials.tsx`:
- Around line 60-64: The current map uses the testimonial.body string as the
React key which can collide when bodies are identical; update the mapping in
testimonials.map to use a stable unique identifier (e.g., testimonial.id) for
the SwiperSlide key, falling back to a combo like `${testimonial.id ||
testimonial.author}-${index}` or the index only if no id exists; modify the
Testimonial data shape to include an id when possible and update the SwiperSlide
key prop and any related usages of Testimonial to reference that id.
- Line 2: The import of useRef is unused because the code uses React.useRef; fix
by making the usage and import consistent—either remove the named import useRef
from the import list and keep React.useRef usage, or change React.useRef to
useRef and keep the named import; update the import/export line accordingly and
remove the unused identifier so there are no lint warnings related to useRef
(reference symbols: useRef import and React.useRef usage in this component).
In `@apps/site/src/components/careers/worldmap.module.css`:
- Around line 1-189: Extract repeated color values into CSS custom properties on
the .worldmap selector (e.g., --fill-primary, --fill-secondary, --fill-accent)
and replace the hardcoded rgb(...) values used in the .worldmap.active
.svg-elem-2, .svg-elem-3, .svg-elem-4, .svg-elem-5, etc. selectors with those
variables (var(--fill-...)); keep the transition and timing on .worldmap
.svg-elem-N as-is so only the fill values are centralized for maintainability.
In `@apps/site/src/components/navigation-wrapper.tsx`:
- Around line 6-20: You defined a local Link interface in navigation-wrapper.tsx
that duplicates the shared type; replace it by importing the canonical Link type
from `@prisma-docs/ui/components/web-navigation` (or the exported name used there)
and remove the local interface declaration so the component uses the imported
Link type (update any usages in this file to reference the imported symbol).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41db50da-e88a-40a5-ae56-7e2ad43901ea
⛔ Files ignored due to path filters (6)
apps/site/public/illustrations/careers/hero_lines.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/hero_lines_light.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/hero_lines_mobile.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/hero_lines_mobile_light.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/world_map_dots.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
apps/site/package.jsonapps/site/public/photos/careers/masonry_1.webpapps/site/public/photos/careers/masonry_10.webpapps/site/public/photos/careers/masonry_11.webpapps/site/public/photos/careers/masonry_12.webpapps/site/public/photos/careers/masonry_13.webpapps/site/public/photos/careers/masonry_14.webpapps/site/public/photos/careers/masonry_2.webpapps/site/public/photos/careers/masonry_3.webpapps/site/public/photos/careers/masonry_4.webpapps/site/public/photos/careers/masonry_5.webpapps/site/public/photos/careers/masonry_6.webpapps/site/public/photos/careers/masonry_7.webpapps/site/public/photos/careers/masonry_8.webpapps/site/public/photos/careers/masonry_9.webpapps/site/src/app/careers/page.tsxapps/site/src/app/global.cssapps/site/src/app/layout.tsxapps/site/src/components/careers/Challenges.tsxapps/site/src/components/careers/Flexible.tsxapps/site/src/components/careers/WorldMap.tsxapps/site/src/components/careers/challenges.module.cssapps/site/src/components/careers/flexible.module.cssapps/site/src/components/careers/masonry-client.tsxapps/site/src/components/careers/masonry.tsxapps/site/src/components/careers/open-roles.tsxapps/site/src/components/careers/stats-list.tsxapps/site/src/components/careers/testimonials.tsxapps/site/src/components/careers/worldmap.module.cssapps/site/src/components/constants.tsapps/site/src/components/homepage/card-section/card-section.tsxapps/site/src/components/navigation-wrapper.tsxapps/site/src/types/react-responsive-masonry.d.tspackages/ui/src/components/navigation-menu.tsxpackages/ui/src/components/quote.tsxpackages/ui/src/components/web-navigation.tsx
Contributor
|
Hi @carlagn Cuuld we try to have same height for the testimonials boxes ? I found this visual a bit "old" ... but just a nit honestly as there nothing maybe to about it right now. |
carlagn
force-pushed
the
feat/DR-7729-careers
branch
from
bf03955 to
683bf45
Compare
Update build issues
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/site/src/app/careers/page.tsx (1)
156-159:⚠️ Potential issue | 🟠 MajorUse careers-specific metadata instead of homepage metadata constants.
Lines 156–159 currently reuse homepage SEO values, which weakens careers page indexing and snippet relevance.
Suggested fix
export const metadata: Metadata = { - title: SITE_HOME_TITLE, - description: SITE_HOME_DESCRIPTION, + title: "Careers at Prisma | Join Our Team", + description: + "Help us empower developers to build data-driven applications. Explore open roles, values, and benefits at Prisma.", };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 156 - 159, The careers page currently exports metadata using homepage constants (export const metadata: Metadata and SITE_HOME_TITLE / SITE_HOME_DESCRIPTION); replace those with careers-specific SEO values by importing or defining dedicated constants (e.g., CAREERS_TITLE and CAREERS_DESCRIPTION) and use them in the metadata export so the careers page has its own title and description rather than reusing SITE_HOME_*.apps/site/src/components/careers/stats-list.tsx (1)
22-22:⚠️ Potential issue | 🟡 MinorGuard
e.subbeforetoLowerCase()to avoid render-time crashes.Line 22 can throw if
subis missing/null, which would break the component render path.Suggested fix
- {e.sub.toLowerCase() === "remote" && `%`} + {e.sub?.toLowerCase() === "remote" && `%`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/stats-list.tsx` at line 22, The render uses e.sub.toLowerCase() which can crash if e.sub is null/undefined; update the StatsList rendering logic (where e is iterated) to guard e.sub before calling toLowerCase()—for example check typeof e.sub === 'string' or use optional chaining (e.sub?.toLowerCase()) and handle the falsy case (skip the comparison or render a default) so the expression `{e.sub.toLowerCase() === "remote" && `%`}` is safe.
🧹 Nitpick comments (1)
apps/site/src/app/careers/page.tsx (1)
286-289: Convert static HTML benefits to JSX to improve maintainability and eliminate unnecessary dangerouslySetInnerHTML usage.The benefits content is entirely hardcoded and static, using only simple
<b>tags for bolding. While the security risk is minimal here since the content is trusted, JSX renders this pattern more directly and eliminates the cognitive overhead of parsing HTML strings. The refactoring is straightforward: convert thetextstrings to JSX elements in the data structure, then render them directly without dangerouslySetInnerHTML.Suggested refactor direction
const benefits = [ { icon: "fa-regular fa-arrow-trend-up", - text: "<b>Stock options package</b> with a maximum exercise period of 10 years after grant", + text: <> + <b>Stock options package</b> with a maximum exercise period of 10 years after grant + </>, }, // ... other benefits ];- <p - className="text-lg" - dangerouslySetInnerHTML={{ __html: benefit.text }} - /> + <p className="text-lg">{benefit.text}</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 286 - 289, The current rendering uses dangerouslySetInnerHTML on benefit.text (in apps/site/src/app/careers/page.tsx) for static strings that only include simple <b> tags; change the data model to store the content as JSX (e.g., a new property like content or make text a ReactNode) and update the render to use {benefit.content} (or {benefit.text} if you convert it to JSX) instead of dangerouslySetInnerHTML on the <p className="text-lg"> element; ensure any bolding is done with <b> or <strong> JSX nodes in the data and update any consumers of the benefits array accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/site/src/app/careers/page.tsx`:
- Around line 156-159: The careers page currently exports metadata using
homepage constants (export const metadata: Metadata and SITE_HOME_TITLE /
SITE_HOME_DESCRIPTION); replace those with careers-specific SEO values by
importing or defining dedicated constants (e.g., CAREERS_TITLE and
CAREERS_DESCRIPTION) and use them in the metadata export so the careers page has
its own title and description rather than reusing SITE_HOME_*.
In `@apps/site/src/components/careers/stats-list.tsx`:
- Line 22: The render uses e.sub.toLowerCase() which can crash if e.sub is
null/undefined; update the StatsList rendering logic (where e is iterated) to
guard e.sub before calling toLowerCase()—for example check typeof e.sub ===
'string' or use optional chaining (e.sub?.toLowerCase()) and handle the falsy
case (skip the comparison or render a default) so the expression
`{e.sub.toLowerCase() === "remote" && `%`}` is safe.
---
Nitpick comments:
In `@apps/site/src/app/careers/page.tsx`:
- Around line 286-289: The current rendering uses dangerouslySetInnerHTML on
benefit.text (in apps/site/src/app/careers/page.tsx) for static strings that
only include simple <b> tags; change the data model to store the content as JSX
(e.g., a new property like content or make text a ReactNode) and update the
render to use {benefit.content} (or {benefit.text} if you convert it to JSX)
instead of dangerouslySetInnerHTML on the <p className="text-lg"> element;
ensure any bolding is done with <b> or <strong> JSX nodes in the data and update
any consumers of the benefits array accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33d3eacd-66ca-4284-95fb-f86f113332e9
⛔ Files ignored due to path filters (3)
apps/site/public/illustrations/careers/hero_lines_mobile.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/world_map_dots.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/site/src/app/careers/page.tsxapps/site/src/components/careers/Challenges.tsxapps/site/src/components/careers/Flexible.tsxapps/site/src/components/careers/WorldMap.tsxapps/site/src/components/careers/challenges.module.cssapps/site/src/components/careers/flexible.module.cssapps/site/src/components/careers/masonry-client.tsxapps/site/src/components/careers/open-roles.tsxapps/site/src/components/careers/stats-list.tsxapps/site/src/components/careers/worldmap.module.cssapps/site/src/components/constants.tsapps/site/src/types/react-responsive-masonry.d.tspackages/ui/src/components/quote.tsxpackages/ui/src/components/web-navigation.tsx
✅ Files skipped from review due to trivial changes (7)
- packages/ui/src/components/web-navigation.tsx
- apps/site/src/components/constants.ts
- apps/site/src/components/careers/masonry-client.tsx
- apps/site/src/components/careers/worldmap.module.css
- apps/site/src/components/careers/flexible.module.css
- apps/site/src/types/react-responsive-masonry.d.ts
- apps/site/src/components/careers/challenges.module.css
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ui/src/components/quote.tsx
- apps/site/src/components/careers/open-roles.tsx
- apps/site/src/components/careers/Flexible.tsx
ArthurGamby
previously approved these changes
Mar 31, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/site/src/app/careers/page.tsx (4)
166-175: Consider empty alt for decorative images.These background SVG illustrations are purely decorative. Screen readers will announce "Hero lines" which adds noise. For decorative images, an empty
alt=""is preferred to indicate they should be skipped.♻️ Suggested change
<img src="/illustrations/careers/hero_lines.svg" - alt="Hero lines" + alt="" className="w-full h-full object-cover object-bottom hidden dark:block" /> <img src="/illustrations/careers/hero_lines_light.svg" - alt="Hero lines" + alt="" className="w-full h-full object-cover object-bottom block dark:hidden" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 166 - 175, The two decorative SVG <img> elements in page.tsx (src="/illustrations/careers/hero_lines.svg" and src="/illustrations/careers/hero_lines_light.svg") should not expose accessible text; change their alt attributes to an empty string (alt="") and add aria-hidden="true" so screen readers skip them, keeping the existing className and responsive dark/light logic intact.
286-289: Consider using React elements instead ofdangerouslySetInnerHTML.While the static analysis tool flags this as a potential XSS risk, the content here is hardcoded in the source file—so there's no immediate vulnerability. That said,
dangerouslySetInnerHTMLcan become a footgun if someone later refactors to pull this data from an external source.A cleaner approach: structure the benefits data to use React elements directly.
♻️ Suggested refactor
const benefits = [ { icon: "fa-regular fa-arrow-trend-up", - text: "<b>Stock options package</b> with a maximum exercise period of 10 years after grant", + title: "Stock options package", + description: "with a maximum exercise period of 10 years after grant", }, // ... similar for other items ];Then in JSX:
- <p - className="text-lg" - dangerouslySetInnerHTML={{ __html: benefit.text }} - /> + <p className="text-lg"> + <b>{benefit.title}</b> {benefit.description} + </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 286 - 289, Replace the use of dangerouslySetInnerHTML for benefit.text with React elements by changing the benefits data shape from strings to React nodes and rendering them directly (e.g., update the benefits array/constant where benefit is defined to use JSX/React elements instead of HTML strings), then render with <p className="text-lg">{benefit.text}</p> (or rename property to benefit.content if helpful) in page.tsx so you remove dangerouslySetInnerHTML while keeping the same styling and semantics.
295-299:OpenRolescomponent lacks loading and error state handling.The component fetches jobs from the Rippling API in
useEffect(line 79) but has no loading or error states. The initialjobsstate is an empty object (line 56), and there's no conditional rendering to show feedback while the fetch is in progress or if it fails.The fetch call (lines 59-61) also has no error handling—if the API fails, the component silently renders nothing, leaving users without any indication of what's happening.
To improve this, the component should:
- Track loading state during the fetch
- Display a skeleton loader or spinner while jobs are being fetched
- Add a
.catch()block to handle API failures- Show an error message or retry option if the request fails
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 295 - 299, OpenRoles currently initializes jobs as an empty object and fetches data in useEffect without loading/error handling; update the component to add useState hooks for loading (e.g., isLoading) and error (e.g., fetchError), change jobs to a nullable array or [] initial type, and refactor the fetch logic (the existing useEffect and fetch call) to set isLoading=true before the request, setJobs(data) on success, set fetchError on .catch(...) and ensure a finally that sets isLoading=false. In the component JSX (inside OpenRoles) conditionally render a spinner/skeleton when isLoading, an error message with a retry button that re-runs the fetch when fetchError is set, and the normal jobs list only when !isLoading && !fetchError && jobs has data.
17-32: UpdateMasonryPictto use the providedimageAltproperty for accessibility.Each image object in
masonryImagesincludes a meaningfulimageAltproperty, but theMasonryPictcomponent generates generic alt text (img-${idx}) instead of using it. This misses an opportunity to provide more descriptive accessibility information. Update the component to accept and render the alt text from the images array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 17 - 32, The MasonryPict component currently renders images with a generic alt like `img-${idx}` instead of using the provided descriptive alt text; update MasonryPict to accept an alt prop (or read `imageAlt`) and render that value for the img alt attribute when mapping `masonryImages`, ensuring you pass `image.imageAlt` from the `masonryImages` array to MasonryPict (or into the <img> inside the mapping) and fall back to an empty string or a safe default if `imageAlt` is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/careers/page.tsx`:
- Around line 255-269: The h3 headings for "Solidarity" and "Accountability" are
missing the font-sans-display class, causing inconsistent typography with the
other value headings; update the h3 elements that render "Solidarity" and
"Accountability" to include the same font-sans-display class used on
"Transparency" and "Curiosity" (ensure the h3s inside the JSX blocks that
contain Action components and the text "Solidarity" and "Accountability" are
updated), and quickly scan the other value-heading h3s to confirm all use
font-sans-display for consistent styling.
- Around line 283-285: The JSX for the icon element is duplicating the
"fa-regular" class because benefit.icon already includes that prefix; update the
<i> element's className construction (currently className={`fa-regular
${benefit.icon} text-3xl text-foreground-orm`}) to avoid adding "fa-regular"
twice, or alternatively normalize the data by removing the "fa-regular" prefix
from each benefit.icon value so the template can safely prepend it; locate the
icon <i> element and the benefit.icon values and apply one of these two fixes so
the final class string contains a single "fa-regular".
---
Nitpick comments:
In `@apps/site/src/app/careers/page.tsx`:
- Around line 166-175: The two decorative SVG <img> elements in page.tsx
(src="/illustrations/careers/hero_lines.svg" and
src="/illustrations/careers/hero_lines_light.svg") should not expose accessible
text; change their alt attributes to an empty string (alt="") and add
aria-hidden="true" so screen readers skip them, keeping the existing className
and responsive dark/light logic intact.
- Around line 286-289: Replace the use of dangerouslySetInnerHTML for
benefit.text with React elements by changing the benefits data shape from
strings to React nodes and rendering them directly (e.g., update the benefits
array/constant where benefit is defined to use JSX/React elements instead of
HTML strings), then render with <p className="text-lg">{benefit.text}</p> (or
rename property to benefit.content if helpful) in page.tsx so you remove
dangerouslySetInnerHTML while keeping the same styling and semantics.
- Around line 295-299: OpenRoles currently initializes jobs as an empty object
and fetches data in useEffect without loading/error handling; update the
component to add useState hooks for loading (e.g., isLoading) and error (e.g.,
fetchError), change jobs to a nullable array or [] initial type, and refactor
the fetch logic (the existing useEffect and fetch call) to set isLoading=true
before the request, setJobs(data) on success, set fetchError on .catch(...) and
ensure a finally that sets isLoading=false. In the component JSX (inside
OpenRoles) conditionally render a spinner/skeleton when isLoading, an error
message with a retry button that re-runs the fetch when fetchError is set, and
the normal jobs list only when !isLoading && !fetchError && jobs has data.
- Around line 17-32: The MasonryPict component currently renders images with a
generic alt like `img-${idx}` instead of using the provided descriptive alt
text; update MasonryPict to accept an alt prop (or read `imageAlt`) and render
that value for the img alt attribute when mapping `masonryImages`, ensuring you
pass `image.imageAlt` from the `masonryImages` array to MasonryPict (or into the
<img> inside the mapping) and fall back to an empty string or a safe default if
`imageAlt` is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a0debc90-d613-4ebd-ba6a-00a7292a149a
📒 Files selected for processing (2)
apps/site/src/app/careers/page.tsxapps/site/src/components/homepage/card-section/card-section.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/site/src/components/homepage/card-section/card-section.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/site/src/components/careers/open-roles.tsx (3)
14-14: Consider adding proper TypeScript types for props.Using
anyfor the props loses the benefits of TypeScript's type checking. A simple interface would catch misuse at compile time.interface OpenRolesMenuProps { setFilter: (value: string) => void; filters: string[]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/open-roles.tsx` at line 14, Replace the loose any props on the OpenRolesMenu component with a concrete props interface: create an OpenRolesMenuProps interface and type the component signature (OpenRolesMenu) to accept it, ensuring setFilter is typed as a function (e.g., (value: string) => void) and filters is typed as a string[] so TypeScript can validate callers and usages inside the component.
91-93: Consider movingfetchJobsinside the effect or memoizing it.The
react-hooks/exhaustive-depslint rule would flag thatfetchJobsis referenced but not in the dependency array. Since the function is defined fresh each render, this pattern can be confusing. Moving the async logic directly into the effect (or usinguseCallback) makes the data flow clearer.♻️ Suggested pattern
React.useEffect(() => { - fetchJobs(); + async function fetchJobs() { + // ... existing implementation + } + fetchJobs(); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/open-roles.tsx` around lines 91 - 93, The effect currently calls the outer fetchJobs function which is re-created each render; either move the async logic inside React.useEffect (define an async function like fetchAndSet within the effect and immediately invoke it) or memoize the existing fetchJobs with React.useCallback so it can safely be referenced from the dependency array; if you move it inside the effect, include any used vars in the dependency array (or leave it empty only if truly static) and if you use useCallback, update React.useEffect to include fetchJobs in its dependency list to satisfy react-hooks/exhaustive-deps.
64-66: Avoid mutating the original job object insidemap.Directly assigning
job.dept = ...mutates the objects from the API response. While it works here, returning a new object is the safer, more predictable pattern—especially if this data is ever shared or cached.♻️ Suggested refactor
const rjobsWithDept = rdata .map((job: any) => { - job.dept = job.department.label; - return job; + return { ...job, dept: job.department.label }; })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/open-roles.tsx` around lines 64 - 66, The map callback is mutating each API job object by assigning job.dept directly; change the map to return a new object instead (e.g., use the spread pattern to return {...job, dept: job.department.label}) so you don't modify the original API response—locate the .map((job: any) => { job.dept = job.department.label; return job; }) and replace it with a non-mutating return that sets dept from job.department.label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/components/careers/open-roles.tsx`:
- Around line 112-114: The code accesses job.workLocation.label.split(...) but
only guards job.workLocation, so if label is null/undefined it will throw;
update the expression in the open-roles rendering to use optional chaining on
label (e.g. job.workLocation?.label?.split(" (")[0]) or a conditional fallback
(e.g. || "") so label is checked before calling split; adjust the JSX where job,
workLocation, and label are referenced to use these safe checks.
- Around line 98-99: The code may call .map on an undefined jobs[filter]; update
the rendering logic in the OpenRoles component to guard against missing keys by
filtering the selected list to existing job groups before mapping (e.g., replace
(filter === "All" ? Object.keys(jobs) : [filter]) with a safe variant like
(filter === "All" ? Object.keys(jobs) : [filter]).filter(key => jobs[key]) ), or
alternatively reset/normalize filter when jobs keys change; ensure all usages of
jobs[filter].map are only executed after confirming jobs[filter] is defined.
- Line 61: The code constructs the Rippling URL using BOARD_ID (read from
process.env.NEXT_PUBLIC_BOARD_ID) without checking it; add a guard where the URL
is built in open-roles.tsx (the place that reads BOARD_ID) to validate BOARD_ID
at runtime—if it's falsy, log a clear error and either throw/return early (e.g.,
return empty job list or render a fallback) instead of constructing
`.../board/undefined/jobs`; alternatively, provide a sensible fallback or
fail-fast during build by validating process.env.NEXT_PUBLIC_BOARD_ID at module
initialization and surfacing a descriptive error so the request URL is never
created with "undefined".
- Around line 60-62: The fetch in open-roles.tsx assigns rdata directly from
res.json() without checking HTTP status; update the fetch logic around the const
rdata = await fetch(...).then((res) => res.json()) to first inspect the Response
(use res.ok or res.status) and if not ok throw or return a clear error object
before calling res.json(); additionally, when consuming rdata later (the
subsequent .map usage) guard with Array.isArray(rdata) or normalize the response
to an empty array to avoid runtime errors when the API returns an error payload.
---
Nitpick comments:
In `@apps/site/src/components/careers/open-roles.tsx`:
- Line 14: Replace the loose any props on the OpenRolesMenu component with a
concrete props interface: create an OpenRolesMenuProps interface and type the
component signature (OpenRolesMenu) to accept it, ensuring setFilter is typed as
a function (e.g., (value: string) => void) and filters is typed as a string[] so
TypeScript can validate callers and usages inside the component.
- Around line 91-93: The effect currently calls the outer fetchJobs function
which is re-created each render; either move the async logic inside
React.useEffect (define an async function like fetchAndSet within the effect and
immediately invoke it) or memoize the existing fetchJobs with React.useCallback
so it can safely be referenced from the dependency array; if you move it inside
the effect, include any used vars in the dependency array (or leave it empty
only if truly static) and if you use useCallback, update React.useEffect to
include fetchJobs in its dependency list to satisfy react-hooks/exhaustive-deps.
- Around line 64-66: The map callback is mutating each API job object by
assigning job.dept directly; change the map to return a new object instead
(e.g., use the spread pattern to return {...job, dept: job.department.label}) so
you don't modify the original API response—locate the .map((job: any) => {
job.dept = job.department.label; return job; }) and replace it with a
non-mutating return that sets dept from job.department.label.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7f2f562-9095-42e0-baac-aa236b307182
📒 Files selected for processing (1)
apps/site/src/components/careers/open-roles.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/site/src/app/careers/page.tsx (1)
307-310: Refactor benefit copy to JSX instead ofdangerouslySetInnerHTML.The benefits array contains only static markup with simple
<b>tags for emphasis. Since this is hardcoded content in your source, there's no meaningful security concern—butdangerouslySetInnerHTMLcreates unnecessary friction here. Restructure the benefits data to use JSX elements directly, which aligns with how the rest of your component composition works and is more maintainable long-term.For example, instead of:
{ icon: "fa-regular fa-arrow-trend-up", text: "<b>Stock options package</b> with..." }Model it as:
{ icon: "fa-regular fa-arrow-trend-up", content: <><b>Stock options package</b> with...</> }Then render it as
{benefit.content}in the JSX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 307 - 310, Refactor the benefits data and rendering to avoid dangerouslySetInnerHTML: update the benefits array items (used where `benefits` is defined) to replace the `text` string with a `content` JSX node (e.g., <><b>…</b> plain text</>), update any related TypeScript type/interface to accept JSX.Element instead of string, and change the render in `page.tsx` from using `dangerouslySetInnerHTML={{ __html: benefit.text }}` to rendering `{benefit.content}` inside the <p> (or appropriate) element; remove the `dangerouslySetInnerHTML` usage and ensure existing bold/inline markup is expressed as JSX within the new `content` fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/careers/page.tsx`:
- Line 178: The metadata object sets twitter.images to a nested array (images:
[["/og/og-careers.png"]]) which is invalid for Next.js; locate the metadata
block in page.tsx (the property named images under twitter) and flatten the
value to a single-level string or string[] (e.g., images: ["/og/og-careers.png"]
or images: "/og/og-careers.png") so it matches the Metadata API types and
removes the extra array nesting.
---
Nitpick comments:
In `@apps/site/src/app/careers/page.tsx`:
- Around line 307-310: Refactor the benefits data and rendering to avoid
dangerouslySetInnerHTML: update the benefits array items (used where `benefits`
is defined) to replace the `text` string with a `content` JSX node (e.g.,
<><b>…</b> plain text</>), update any related TypeScript type/interface to
accept JSX.Element instead of string, and change the render in `page.tsx` from
using `dangerouslySetInnerHTML={{ __html: benefit.text }}` to rendering
`{benefit.content}` inside the <p> (or appropriate) element; remove the
`dangerouslySetInnerHTML` usage and ensure existing bold/inline markup is
expressed as JSX within the new `content` fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7bdafe1-2515-4182-9c22-a4397061bfe2
⛔ Files ignored due to path filters (1)
apps/site/public/og/og-careers.pngis excluded by!**/*.png
📒 Files selected for processing (1)
apps/site/src/app/careers/page.tsx
Merged
This was referenced Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements