Skip to content

Conversation

pomarec
Copy link

@pomarec pomarec commented Jun 12, 2025

πŸ”— Linked issue

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Adds apiOnly() on a group of routes, applying apiOnly() on all its resources.
It is very handy on some of my projects as it gives you a a clearer routes.ts.

πŸ“ Checklist

@thetutlage
Copy link
Member

Hey, may I know what purpose does this PR solve? Because, it could be confusing to understand how apiOnly is applied to the group routes when there are other routes as well (non resourceful routes).

@thetutlage
Copy link
Member

Closing since not actionable

@thetutlage thetutlage closed this Jun 17, 2025
@pomarec
Copy link
Author

pomarec commented Jun 17, 2025

@thetutlage
As I said, the purpose is, in my case, "it gives you a a clearer routes.ts."

router
  .group(() => {
    router.resource('conversation', ConversationController).except(['update', 'destroy'])
    router.resource('message', modelController(MessageService)).except(['update', 'destroy'])
    router.resource('notification', NotificationController).except(['store', 'update', 'destroy'])
  })
  .use(middleware.auth())
  .apiOnly()

is clearer than

router
  .group(() => {
    router.resource('conversation', ConversationController).except(['update', 'destroy']).apiOnly()
    router
      .resource('message', modelController(MessageService))
      .except(['update', 'destroy'])
      .apiOnly()
    router
      .resource('notification', NotificationController)
      .except(['store', 'update', 'destroy'])
      .apiOnly()
  })
  .use(middleware.auth())

I agree with the confusion it can lead to though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants