refactor: Move data-table hooks/components from /features to /web#27228
refactor: Move data-table hooks/components from /features to /web#27228
Conversation
…rom packages/features to apps/web/modules Co-Authored-By: benny@cal.com <sldisek783@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: benny@cal.com <sldisek783@gmail.com>
| } from "@calcom/app-store/_components/EventTypeAppCardInterface"; | ||
| import type { EventTypeAppsList } from "@calcom/app-store/utils"; | ||
| import useAppsData from "@calcom/web/modules/apps/hooks/useAppsData"; | ||
| import useAppsData from "@calcom/features/apps/hooks/useAppsData"; |
There was a problem hiding this comment.
bonus: I moved useAppsData to /features because Atoms imports it
caae127 to
fda9578
Compare
02c9924 to
7ed3cc3
Compare
E2E results are ready! |
eunjae-lee
left a comment
There was a problem hiding this comment.
Hey thanks for the work. As you can see, a lot from data table has been moved to apps/web/modules. It's because data-table heavily relies on web-related stuff like query parameters and stuff like that. And now we have only a few utilities remaining in packages/features folder. What if we entirely move everything to apps/web/modules? If we ever want to use DataTable on other than web platform, then we can think about better refactoring this DataTable feature to be more adaptable. WDYT?
Pull request was converted to draft
|
@eunjae-lee I believe |
|
I just re-checked the current status. All the components and hooks are in apps/web/module/data-table, and the types, utilities, server related stuff are in packages/features/data-table. Yeah seems reasonable. Although it might get confusing what to import from where, but with the updated packages/features/data-table/GUIDE.md, it should be alright, I think. |
|
@hbjORbj lint errors |
Co-Authored-By: benny@cal.com <sldisek783@gmail.com>
6ff4430
|
I'm actually working on data-table in another PR that is moving things quite a lot. let me close this PR, and after that PR is done, i will apply this structural change in a follow-up PR. |
Pull request was closed
|
Sure, thanks @eunjae-lee , ping me for review |
What does this PR do?
This PR is part of a larger effort to break down PR #26571 into smaller, reviewable chunks. It moves hooks and components from
packages/features/data-tabletoapps/web/modules/data-tableto support the vertical slice architecture.Updates since last revision
CalendarEventBuilder.test.ts: changedTimeFormat["TWENTY_FOUR_HOUR"]toTimeFormat.TWENTY_FOUR_HOURto satisfy thelint/complexity/useLiteralKeysruleMandatory Tasks (DO NOT REMOVE)
How should this be tested?
This is a refactoring PR that moves code without changing functionality. Verify that:
yarn lintyarn type-check:ciHuman Review Checklist
@calcom/features/data-tableto@calcom/web/modules/data-table/hooksor~/data-table/components)useAppsDatamove to@calcom/features/apps/hooksis intentional (needed for Atoms imports)Link to Devin run: https://app.devin.ai/sessions/5b290a1011c84be1a4bfa146cb313c85
Requested by: benny@cal.com (benny@cal.com) (@hbjORbj)