github-starred/vikunja
2
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-05-05 19:18:16 -05:00
Code Issues 690 Packages Projects Releases 11 Wiki Activity

[GH-ISSUE #533] refactor: services: add compile-time checks for existence of CRUD methods #6226

New Issue
Open
opened 2026-04-20 16:49:06 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-20 16:49:06 -05:00
Owner
Copy Link

Originally created by @vikunja-bot on GitHub (Apr 1, 2025).
Original GitHub issue: https://github.com/go-vikunja/vikunja/issues/533

Original issue by WofWca on 2023-03-06T12:36:34.000Z

Right now every service has all the methods (getAll, create, post), but not all of them are actually implemented by all of the services. There is a runtime check:

6aa02e29b1/src/services/abstractService.ts (L345-L347)

getAll would throw if you were to call it on UserSettingsService:
2b82df5dbd/src/services/userSettings.ts (L4-L8)

would be nice if the property actually didn't exist instead. Or, IMO, better yet, if each CRUD method was exported separately somehow, for better modularity and ease to see what's used (and unused) where (yes I'm an OOP hater). Also the fact that loading is shared for all the CRUD methods is weird to me.

Explicitly listing supported query parameters would also be cool.

Original issue on Gitea

@kolaente commented on 2023-03-06T18:57:21.000Z:

Do you have an idea on how to do this without much boilerplate?

WofWca commented on 2023-03-06T19:04:45.000Z:

I didn't think too much about it, but I'm sure it shouldn't be exceptionally hard.

I was thinking about initially inferring the available methods from the Paths object provided to the constructor (e.g. if there's no getAll property then the getAll method should return never), but I haven't come up with a way to fit it in the class paradigm properly.

@kolaente commented on 2023-03-06T19:07:11.000Z:

I was thinking about initially inferring the available methods from the Paths object provided to the constructor (e.g. if there's no getAll property then the getAll method should return never), but I haven't come up with a way to fit it in the class paradigm properly.

That sounds like it could work (can't say this 100% sure due to my lack of ts knowledge). What's preventing you to fit this in the class paradigm?

@dpschen Do you have any idea about this?

WofWca commented on 2023-03-06T19:09:41.000Z:

What's preventing you to fit this in the class paradigm?

I kinda tried a bit, but then gave up. Should be possible.

dpschen commented on 2023-03-07T15:46:25.000Z:

@konrad Yes. We talked about this a while ago.

I would love to have something like this:

const userApi = useRessource('https://jsonplaceholder.typicode.com',
({createEndpoint}) => ({
  index: createEndpoint<User[]>('GET', 'users')
  search: createEndpoint<User[]>('GET', 'users?:key=:value'),
  create: createEndpoint<User>('POST', 'users'),
  read: createEndpoint<User>('GET', 'users/:id'),
  // note URL-only parameter - :id!
  update: createEndpoint<User>('PUT', 'users/:id!'),
  delete: createEndpoint<User>('DELETE', 'users/:id'),
}), [ log ])

This takes heavy inspiration from: https://stackblitz.com/edit/axios-actions-ts?devToolsHeight=100&file=src%2Flib.ts%2Csrc%2Fmain.ts

Zodiac does a good job. Sadly it's still axios based and I really would like to replace axios / make that fetcher independent. I would prefer using native fetch or something like ky. In v11 they want to allow this though.

Originally created by @vikunja-bot on GitHub (Apr 1, 2025). Original GitHub issue: https://github.com/go-vikunja/vikunja/issues/533 _Original issue by WofWca on 2023-03-06T12:36:34.000Z_ Right now every service has all the methods (`getAll`, `create`, `post`), but not all of them are actually implemented by all of the services. There is a runtime check: https://kolaente.dev/vikunja/frontend/src/commit/6aa02e29b19f9f57620bdf09919df34c363e1f3d/src/services/abstractService.ts#L345-L347 `getAll` would throw if you were to call it on `UserSettingsService`: https://kolaente.dev/vikunja/frontend/src/commit/2b82df5dbd1c8ea921de67842e78b6f9fad077af/src/services/userSettings.ts#L4-L8 would be nice if the property actually didn't exist instead. Or, IMO, better yet, if each CRUD method was exported separately somehow, for better modularity and ease to see what's used (and unused) where (yes I'm an OOP hater). Also the fact that [`loading`](https://kolaente.dev/vikunja/frontend/src/commit/6aa02e29b19f9f57620bdf09919df34c363e1f3d/src/services/abstractService.ts#L50) is shared for all the CRUD methods is weird to me. Explicitly listing supported query parameters would also be cool. [Original issue on Gitea](https://kolaente.dev/vikunja/vikunja/issues/2018) --- _**@kolaente** commented on 2023-03-06T18:57:21.000Z_: Do you have an idea on how to do this without much boilerplate? --- _**WofWca** commented on 2023-03-06T19:04:45.000Z_: I didn't think too much about it, but I'm sure it shouldn't be exceptionally hard. I was thinking about initially inferring the available methods from the `Paths` object provided to the constructor (e.g. if there's no `getAll` property then the `getAll` method should return `never`), but I haven't come up with a way to fit it in the class paradigm properly. --- _**@kolaente** commented on 2023-03-06T19:07:11.000Z_: > I was thinking about initially inferring the available methods from the `Paths` object provided to the constructor (e.g. if there's no `getAll` property then the `getAll` method should return `never`), but I haven't come up with a way to fit it in the class paradigm properly. That sounds like it could work (can't say this 100% sure due to my lack of ts knowledge). What's preventing you to fit this in the class paradigm? @dpschen Do you have any idea about this? --- _**WofWca** commented on 2023-03-06T19:09:41.000Z_: > What's preventing you to fit this in the class paradigm? I kinda tried a bit, but then gave up. Should be possible. --- _**dpschen** commented on 2023-03-07T15:46:25.000Z_: @konrad Yes. We talked about this a while ago. I would love to have something like this: ```ts const userApi = useRessource('https://jsonplaceholder.typicode.com', ({createEndpoint}) => ({ index: createEndpoint<User[]>('GET', 'users') search: createEndpoint<User[]>('GET', 'users?:key=:value'), create: createEndpoint<User>('POST', 'users'), read: createEndpoint<User>('GET', 'users/:id'), // note URL-only parameter - :id! update: createEndpoint<User>('PUT', 'users/:id!'), delete: createEndpoint<User>('DELETE', 'users/:id'), }), [ log ]) ``` This takes heavy inspiration from: https://stackblitz.com/edit/axios-actions-ts?devToolsHeight=100&file=src%2Flib.ts%2Csrc%2Fmain.ts [Zodiac](https://github.com/ecyrbe/zodios#declare-your-api-with-zodios) does a good job. Sadly it's still axios based and I really would like to replace axios / make that fetcher independent. I would prefer using native fetch or something like [ky](https://github.com/sindresorhus/ky). In v11 they want to allow this though.
GiteaMirror added the area/frontendarea/internal-code labels 2026-04-20 16:49:06 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/api
area/attachments
area/auth
area/avatars
area/backup-restore
area/caldav
area/calendar-view
area/comments
area/config
area/database
area/desktop
area/docker
area/email
area/favorites
area/filters
area/frontend
area/gantt
area/i18n
area/import-export
area/internal-code
area/kanban
area/labels
area/list-view
area/mobile
area/notifications
area/permissions
area/projects
area/pwa
area/recurring-tasks
area/reminders
area/search
area/shortcuts
area/subtasks
area/sync
area/table-view
area/task-editor
area/task-metadata
area/task-relations
area/teams
area/theming
area/time-tracking
area/typesense
area/views
area/webhooks
bug
changes requested
concern/accessibility
concern/performance
concern/regression
concern/ux
confirmed
db/mysql
dependencies
enhancement
good first issue
help wanted
integration/inbound
integration/outbound
kind/bug
kind/feature
needs reproduction
pull-request
Mirrored from GitHub Pull Request
 question
security
support
upstream issue
waiting for reply
wontfix
No Label area/frontend area/internal-code
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/vikunja#6226
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?