refactor: move parcel dashboard to /apps#1957
Conversation
- Move app/ → apps/dashboard-parcel/ - Update package.json and gulpfile.js paths - Fix index.html relative path - Update deployment documentation
- Move test/ → apps/dashboard-parcel/test/ (E2E tests and utilities) - Move utils/link-next.sh → apps/dashboard-parcel/utils/ (dev workflow) - Move README.md → apps/dashboard-parcel/README.md (app documentation) - Keep static/ in root due to library coupling (imported by packages/veda-ui) Updated configurations: - Jest: alias points to apps/dashboard-parcel/test/ - Package.json: alias and pretest:e2e script path updated - TypeScript: path mapping updated - ESLint: Playwright test files path updated - Gulpfile: static file paths corrected - Documentation: deployment guide updated All checks passing: TypeScript, tests, linting, build, dev server
- Restore original README.md to root - Add Component Library suffix to README-lib.md - Update apps/dashboard-parcel/README.md to match root README structure - All documentation links verified and working
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
I've converted this to a draft. There are some issues with the testing setup that need a closer look. |
Moves Playwright config to root to prevent test discovery conflicts in monorepo. Config now scopes to dashboard-parcel E2E tests only. - Add concise documentation - Fix testDir and testMatch to isolate dashboard app E2E tests
|
This is ready for review. |
apps/dashboard-parcel/README.md
Outdated
| @@ -0,0 +1,30 @@ | |||
| # VEDA Dashboard | |||
There was a problem hiding this comment.
Can only parcel specific documentation live in this readme so we're not duplicating readmes.
If we're not planning on persisting parcel, how about just updating the existing readme to reference that parcel will be deprecated and a TODO with ticket number.
Top level docs that reference Parcel which need updates:
- MDX_BLOCKS.md (references parcel specific url builder new URL)
- ARCHITECTURE.md (a lot of parcel specific build notes)
- DEPLOYMENT.md (you updated the path, but this is specific to the parcel build)
- PAGE_OVERRIDES_DEV.md (developer note on parcel type definitions)
- SETUP.md (remove link to parcel env var docs)
Also wondering if the parcel specific folders should be moved here from the top level too?
/parcel-resolver...
/parcel-transfomer...
and the automated parcel bundle reports
| "ts-check": "yarn tsc --noEmit --skipLibCheck", | ||
| "test": "TZ=UTC jest", | ||
| "pretest:e2e": "node test/playwright/generateTestData.js", | ||
| "pretest:e2e": "node apps/dashboard-parcel/test/playwright/generateTestData.js", |
There was a problem hiding this comment.
This would be specific to a parcel only build. Wondering if playwright tests can be general to all builds (vite, next, etc) to prevent the need to duplicate tests that should be build neutral?
|
|
||
| ``` | ||
| PARCEL_BUNDLE_ANALYZER=true yarn parcel build app/index.html | ||
| PARCEL_BUNDLE_ANALYZER=true yarn parcel build apps/dashboard-parcel/index.html |
There was a problem hiding this comment.
See comment above. wondering if parcel specific concerns shouldn't be in top level documentation if we're expecting to support multiple types of dashboard builds. Otherwise, I think we should provide a comment with that indicates deprecation and a TODO with ticket number.
| "targets": { | ||
| "veda-app": { | ||
| "source": "./app/index.html", | ||
| "source": "./apps/dashboard-parcel/index.html", |
There was a problem hiding this comment.
This is also specific to a parcel build. Will there be instructions to update this for other builds (vite, next)?
If not, same as above, I think we should provide a comment that indicates deprecation and a TODO with ticket number.
| "$libs": "~/packages/veda-ui/src/libs", | ||
| "$graphics": "~/packages/veda-ui/graphics", | ||
| "$test": "~/test", | ||
| "$test": "~/apps/dashboard-parcel/test", |
There was a problem hiding this comment.
Same as above, can tests be neutral to builds. If not, can this become parcel-test with a notice for deprecation and a TODO with a ticket number.
There was a problem hiding this comment.
Couple questions,
Are we expecting development on veda-ui to continue? My understanding, was we were "sunsetting" this repo and weren't making substantive changes, just bug fixes. But maybe I'm wrong!
If we are continuing to develop, I'm curious why the dashboard is being updated to support multiple build tools? What is the value? If the intent is to support both parcel temporarily and vite long term, I'd prefer using a folder prefixed with legacy, so it's obvious that parcel should not be used going forward and that ultimately it will be deleted. I'd also have a preference to update the primary readme with something that makes it obvious that the repo is "under construction" and to point to the ADR with the outlined work.
If that's not the intent, how would an engineer choose which build to use? I'm open to understanding this better!
Added a few more comments below, if we're intending to support multiple builds for the dashboard, but my initial instinct is this adds confusion.
Based on the priorities for this year and the Disasters portal launching (which uses next-veda-ui which in turn uses some of the core veda-ui components, like the Data Catalog, E&A and I think others), I think we'll need to continue supporting. My take on this PR is that it paves the way for a "veda-lite" component library within this repo (I brought up this with Slesa during the last DSE frontend sync, on whether we should think about moving some of the EIE components like the layer item etc in veda-ui for easier reuse), we can just use a different bundler like vite, without refactoring old ones in place. We can also copy old ones in the new directory to avoid the same refactor in place and be more quicker in development without worrying that we're breaking legacy instances. That's just my 2 cents. |
Thanks for the context! Two thoughts:
|
- Add Parcel title and migration notice to dashboard-parcel README - Document README-lib.md purpose for library consumers
|
Thanks for the review @ifsimicoded and @dzole0311. This PR is not introducing a multi-builder setup. The On the specific points raised:
On v7 cleanup vs. a new repo, my preference is a dedicated v7 branch in this repo, starting clean and dropping all legacy files, effectively restarting from scratch. This would help keep the project history and context of tickets and PRs. That said, I think this discussion belongs in the v7 ticket #1889. @ifsimicoded Apologies I didn't responded on the inline comments, I think my response would be better organized in a single comment. This is ready for another review, please let me know if we need to include further changes on this. |
👍 Ah, I misunderstood the description! so v6 build and potentially a separate v7 build preserving legacy parcel build. Thanks for clarifying!
👍 Ah ok, the plan would be to rewrite tests for components when we introduce v7.
👍
👍 can this be documented in the ADR or ticketed in github as part of a monorepo or pre-v7 initiative? My worry is a lot of what is in your head about the plan for migration is only in your head, making it difficult for others to understand the plan, contribute, or pick up if your time is taken else where. It would be helpful if there's work that's required to take the pre-v7 to completion, that it be ticketed as [pre-v7] or similar and individuals could review those / reference the tickets in TODO comments in the repo.
👍
👍 |
There was a problem hiding this comment.
Thank you for clarifying!
Requests / Questions:
- Are there any updates that need to be made in consuming repos? Have you tested both submodule inclusion and library inclusion?
- can "Parcel folders at the root: agreed they should move" be documented in the ADR or ticketed in github as part of a monorepo or pre-v7 initiative? My worry is a lot of what is in your head about the plan for migration is only in your head, making it difficult for others to understand the plan, contribute, or pick up if your time is taken else where. If there's additional work that's required to take the pre-v7 to completion, can it be ticketed as [pre-v7] or similar and those could be referenced in TODO comments in the repo.
|
@ifsimicoded thanks for raising this, it's a fair concern. You’re right that parts of this plan aren’t well documented, and ADR 004 didn’t anticipate the parcel issues. It only became more clear during migration files, and I agree that makes things harder to follow. I'd appreciate regrouping on this in our sync and getting your input on how we should move forward, then reflecting what we align for the future of veda-ui. I’ve tested this with |
|
I forgot to say, there are no updates expected downstream. |
Could you update the ADR with the parcel issues that came up, including
Can we do this after we figure out our PI objectives and staffing, and see what additional hours we'll have left? |
This addresses feedback in #1957 by clarifying the approach and implementation status described in ADR 004.
3 participants
Contributes to #1871. Aims to be a step forward in separation of concerns in this repository.
Moves the parcel app to
/appsasdashboard-parcel, allowing future variants (e.g.dashboard-vite,dashboard-nextjs).Only dashboard-specific files were moved. MDX resolver code and content samples remain in root, to keep the scope of this small.
cc @dzole0311 @ifsimicoded