Implement conditional deployment pipeline for monorepo using turbo and pnpm#218
Merged
Implement conditional deployment pipeline for monorepo using turbo and pnpm#218
Conversation
Contributor
Reviewer's GuideThis PR migrates the monorepo from npm to pnpm, refactors the Azure Pipelines build stage to use Turbo for affected builds and conditional deployment, standardizes all workspace packages with consistent manifests and TypeScript references, and introduces a new shared UI core library with Storybook and Vitest integration. Entity relationship diagram for pnpm workspace and catalog dependencieserDiagram
PACKAGE ||--o{ WORKSPACE : contains
WORKSPACE {
string name
string version
string type
string[] files
string[] dependencies
string[] devDependencies
string[] peerDependencies
}
PACKAGE {
string name
string version
}
CATALOG ||--o{ PACKAGE : provides
CATALOG {
string name
string version
}
WORKSPACE ||--o{ CATALOG : uses
Class diagram for the new @cellix/ui-core library structureclassDiagram
class ComponentQueryLoader {
+error: Error | undefined
+errorComponent?: React.JSX.Element
+loading: boolean
+hasData: object | null | undefined
+hasDataComponent: React.JSX.Element
+noDataComponent?: React.JSX.Element
+loadingRows?: number
+loadingComponent?: React.JSX.Element
}
class RequireAuth {
+children: React.JSX.Element
+forceLogin?: boolean
}
class Molecules {
+ComponentQueryLoader
+RequireAuth
}
class Components {
+Molecules
}
class UI_Core {
+Components
}
UI_Core --> Components
Components --> Molecules
Molecules --> ComponentQueryLoader
Molecules --> RequireAuth
Class diagram for standardized workspace package manifestsclassDiagram
class PackageJson {
+name: string
+version: string
+type: string
+files: string[]
+exports: object
+scripts: object
+dependencies: object
+devDependencies: object
+peerDependencies?: object
}
class TsConfig {
+extends: string
+compilerOptions: object
+include: string[]
+exclude: string[]
+references?: object[]
}
PackageJson --> TsConfig : "tsconfig reference"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider extracting the large inline Bash scripts for PNPM setup and artifact packaging into separate version-controlled script files to improve readability and maintainability of the pipeline YAML.
- Instead of manually installing corepack via npm, evaluate using the official PNPM Azure DevOps task or a corepack-enabled pipeline step to simplify your PNPM setup and reduce custom scripting.
- You have multiple hard-coded references to the Turbo cache directory—parameterizing and centralizing that path would help keep cache configuration consistent and easier to update.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the large inline Bash scripts for PNPM setup and artifact packaging into separate version-controlled script files to improve readability and maintainability of the pipeline YAML.
- Instead of manually installing corepack via npm, evaluate using the official PNPM Azure DevOps task or a corepack-enabled pipeline step to simplify your PNPM setup and reduce custom scripting.
- You have multiple hard-coded references to the Turbo cache directory—parameterizing and centralizing that path would help keep cache configuration consistent and easier to update.
## Individual Comments
### Comment 1
<location> `packages/cellix/ui-core/src/components/molecules/require-auth/index.tsx:58-59` </location>
<code_context>
+ </Row>
+ );
+ }
+ if (auth.isAuthenticated) {
+ return props.children;
+ } else if (auth.error) {
+ return <Navigate to="/" />;
+ } else {
</code_context>
<issue_to_address>
**suggestion:** The logic for error handling may mask underlying issues.
Logging the error or displaying an error message before redirecting would help with troubleshooting and improve user experience.
```suggestion
} else if (auth.error) {
// Log the error for troubleshooting
console.error('Authentication error:', auth.error);
// Display an error message to the user before redirecting
setTimeout(() => {
// Redirect after a short delay
window.location.href = '/';
}, 2500);
return (
<Row justify={'center'} style={{ height: '100vh', alignItems: 'center' }}>
<Space size={'large'} direction="vertical" style={{ textAlign: 'center' }}>
<Typography.Title level={2} type="danger">
Authentication Error
</Typography.Title>
<Typography.Text type="danger">
{auth.error.message || 'An unexpected authentication error occurred. Redirecting to home...'}
</Typography.Text>
</Space>
</Row>
);
```
</issue_to_address>
### Comment 2
<location> `packages/cellix/ui-core/src/components/molecules/require-auth/index.tsx:61-62` </location>
<code_context>
+ } else if (auth.error) {
+ return <Navigate to="/" />;
+ } else {
+ redirectUser();
+ return;
+ }
+};
</code_context>
<issue_to_address>
**issue (bug_risk):** Returning undefined from a React component may cause rendering issues.
Return null instead of undefined to avoid runtime warnings and clarify that no UI should be rendered.
</issue_to_address>
### Comment 3
<location> `packages/cellix/ui-core/src/components/molecules/component-query-loader/index.tsx:15-23` </location>
<code_context>
+ if (props.errorComponent) {
+ return props.errorComponent;
+ }
+ message.error(props.error.message);
+ return <Skeleton/>;
+ }
+ if (props.loading) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Calling message.error on every render may cause repeated notifications.
To prevent duplicate error notifications, use a ref or effect to display the error only once per occurrence.
```suggestion
import React, { useEffect, useRef } from "react";
export const ComponentQueryLoader: FC<ComponentQueryLoaderProps> = (props) => {
const lastErrorMessageRef = useRef<string | undefined>(undefined);
useEffect(() => {
if (props.error && props.error.message !== lastErrorMessageRef.current) {
message.error(props.error.message);
lastErrorMessageRef.current = props.error.message;
}
}, [props.error]);
if (props.error) {
if (props.errorComponent) {
return props.errorComponent;
}
return <Skeleton/>;
}
if (props.loading) {
```
</issue_to_address>
### Comment 4
<location> `packages/cellix/ui-core/README.md:39-42` </location>
<code_context>
+## Folder structure
+
+```
+packages/cellix-ui-core/
+├── src/
+│ ├── components/ # UI components organized by atomic design principles
</code_context>
<issue_to_address>
**issue (typo):** Typo in folder path: should be 'packages/cellix/ui-core/'
The documentation currently uses 'packages/cellix-ui-core/', but it should be 'packages/cellix/ui-core/'.
```suggestion
```
packages/cellix/ui-core/
├── src/
│ ├── components/ # UI components organized by atomic design principles
```
</issue_to_address>
### Comment 5
<location> `packages/cellix/ui-core/src/components/molecules/require-auth/require-auth.stories.tsx:10` </location>
<code_context>
+
+type AwaitedReturn<T> = T extends (...args: unknown[]) => Promise<infer R> ? R : never;
+
+// Wrapper that injects a mocked AuthContext matching react-oidc-context shape
+const Wrapper: React.FC<{ auth: Partial<AuthContextProps>; children: React.ReactNode }> = ({ auth, children }) => {
+ // Provide minimal defaults and stub functions to avoid runtime errors
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the stories to use a global Wrapper decorator and a single Template to eliminate repetitive provider setup and reduce boilerplate.
You’ve already centralized most of your “AuthContext” wiring into that top-level `Wrapper`, but each story is still re-defining its own provider inline – which is why the file is ballooning again. You can collapse all of your “simple” stories down to a single template + a decorator, and only keep per-story `render` blocks for truly custom setups (e.g. your `<Routes>` one).
1. Move `Wrapper` into a global decorator that reads `context.args.auth`:
```tsx
// at the bottom of your file, replace your `meta` with:
const meta = {
title: 'UI/Core/Molecules/RequireAuth',
component: RequireAuth,
parameters: { layout: 'padded' },
decorators: [
(Story, context) => (
<Wrapper auth={context.args.auth ?? {}}>
<Story />
</Wrapper>
)
],
} satisfies Meta<typeof RequireAuth>;
export default meta;
```
2. Create a single “Template” that renders your component + `<Child />`:
```tsx
const Template: StoryFn<typeof RequireAuth> = (args) => (
<RequireAuth {...args}>
<Child />
</RequireAuth>
);
```
3. Now your simple stories reduce to <10 lines each:
```tsx
export const Loading = Template.bind({});
Loading.args = {
forceLogin: false,
auth: { isLoading: true },
};
Loading.play = async ({ canvasElement }) => {
const canvas = within(canvasElement);
expect(canvas.queryByText('Private Content')).toBeNull();
await expect(canvas.findByText(/Please wait.../i)).resolves.toBeTruthy();
};
export const Authenticated = Template.bind({});
Authenticated.args = {
forceLogin: false,
auth: { isAuthenticated: true },
};
Authenticated.play = async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(canvas.findByText('Private Content')).resolves.toBeTruthy();
};
```
4. For stories that need extra routing or bespoke instrumentation (ErrorState, NotAuthenticated, etc.), keep using a custom `render:` block – they’ll still pick up your global decorator, so you can delete the local `<Wrapper>` definitions inside them.
This consolidates ~200 lines of repeating boilerplate into one decorator + one template, while preserving all your existing functionality.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
packages/cellix/ui-core/src/components/molecules/require-auth/require-auth.stories.tsx
Show resolved
Hide resolved
…po build pipeline
…vironment parameters and build conditions
…nd documentation files
…tialization logic
…ues for correct configuration
…ues for correct configuration
…e unused test-watch-all script
…ifact preparation and deployment stages
…er across multiple packages
arif-u-ahmed
pushed a commit
that referenced
this pull request
Nov 4, 2025
Implement conditional deployment pipeline for monorepo using turbo and pnpm
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
1 participant
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.
Fixes #215
Summary by Sourcery
Migrate monorepo to pnpm and add a turbo‐driven build pipeline with conditional artifact packaging
New Features:
Enhancements:
Build:
Documentation:
Tests: