Conversation
move src to viewer make sites/app
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase into a monorepo structure with separate packages for the viewer React component (@biongff/vizarr) and a deployment application (sites/app). The viewer package is now published as a library while the app consumes it for deployment.
Key changes:
- Converted from single package to pnpm monorepo workspace
- Created
viewerpackage exporting the Vizarr React component - Created
sites/apppackage for deployment using the viewer component - Updated build configurations to support library builds and app bundling
- Modified CI/CD workflows to build and publish the viewer package
Reviewed changes
Copilot reviewed 24 out of 55 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
vite.config.js |
Simplified root config (removed custom build logic) |
viewer/vite.config.js |
New library build configuration with TypeScript declarations |
viewer/package.json |
Package metadata for published @biongff/vizarr component |
viewer/src/index.tsx |
Exports viewer component and API |
sites/app/vite.config.js |
App build config with dev alias to viewer source |
sites/app/src/main.tsx |
App entry point using the published viewer |
sites/app/src/App.tsx |
App component managing URL state and viewer |
pnpm-workspace.yaml |
Monorepo workspace configuration |
package.json |
Root package scripts for monorepo management |
.releaserc.json |
Updated release config for monorepo versioning |
.github/workflows/*.yml |
Updated CI/CD to build viewer and app separately |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,10 @@ | |||
| export { version } from "../../package.json"; | |||
There was a problem hiding this comment.
Exporting version from the root package.json is incorrect for the viewer package. The viewer has its own package.json at viewer/package.json with version 1.1.1. This export should reference ../package.json (the viewer's package.json) instead of ../../package.json (the root monorepo package.json).
| fileName: (format) => `biongff-vizarr.${format}.js`, | ||
| }, | ||
| rollupOptions: { | ||
| external: ["react", "react-dom", "@mui/material", "@mui/icons-material", "@emotion/react", "@emotion/styled"], |
There was a problem hiding this comment.
The external array is missing several critical dependencies that should also be externalized for the library build. Dependencies like deck.gl, @deck.gl/*, jotai, zarrita, @zarrita/storage, and @hms-dbmi/viv are listed as regular dependencies in viewer/package.json but are not marked as external. These should either be moved to peerDependencies or added to the external array to avoid bundling them into the library output.
| [ | ||
| "@semantic-release/exec", | ||
| { | ||
| "prepareCmd": "pnpm -r exec npm version ${nextRelease.version} --no-git-tag-version" |
There was a problem hiding this comment.
The prepareCmd uses pnpm -r exec which will run the version command in all workspace packages including the root. However, the root package (biongff-monorepo) should not have the same version as the published viewer package. Consider using a more targeted approach like pnpm --filter viewer exec npm version ${nextRelease.version} --no-git-tag-version or filtering out the root package.
| "prepareCmd": "pnpm -r exec npm version ${nextRelease.version} --no-git-tag-version" | |
| "prepareCmd": "pnpm --filter viewer exec npm version ${nextRelease.version} --no-git-tag-version" |
| pre-release-tag: "dev" | ||
|
|
||
| - name: Sync package versions | ||
| run: pnpm -r exec npm version ${{ steps.pre-release-version.outputs.pre-release-version }} --no-git-tag-version |
There was a problem hiding this comment.
Similar to the release config, this command will update versions in all workspace packages including the root. The root biongff-monorepo package is not published and should not have its version synchronized with the viewer package. Consider filtering to only update the viewer package: pnpm --filter viewer exec npm version ...
| run: pnpm -r exec npm version ${{ steps.pre-release-version.outputs.pre-release-version }} --no-git-tag-version | |
| run: pnpm --filter viewer exec npm version ${{ steps.pre-release-version.outputs.pre-release-version }} --no-git-tag-version |
|
@dannda Have the workflow file changes been tested or are you planning on testing those after merging to the respective branches? It might be useful to do a pre-release for this PR in some form so that the api can be tested before merging. |
✅ Deploy Preview for biongff-vizarr ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
davehorsfall
left a comment
davehorsfall
left a comment
There was a problem hiding this comment.
The restructure looks good. Merging to dev and testing pre-release workflow.
3 participants
Description
make monorepo with packages viewer (component) and sites/app for deployment
export react component and api
monorepo follows a single version for simplicity
Fixes #12
Related to #15 as component expects a sources prop as an array of strings, and sites/app gets all source search params
Type of change
Checklist
npm test)pre-commit run --all-files)