Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRAFT: baseless nested routers working (appx +50 bytes) #208

Closed
wants to merge 25 commits into from

Conversation

kwhitley
Copy link
Owner

@kwhitley kwhitley commented Dec 21, 2023

Description

This adds a new way to nest routers, by passing the entire router as a handler. With this method, child routers do not need explicit base paths set.

const child = Router().get('/', () => 'child')

const parent = Router()
                 .get('/', () => 'parent')
                 .all('/child/*', child)

Notes

  • This will only work with simple prefixes, not ones like /child/:foo/*. For that to work, specify the base path in the child router as before.
  • Specifying the base path of child routers will still be faster, as no per-request manipulation needs to be done

Type of Change (select one and follow subtasks)

  • Documentation (README.md)
  • Maintenance or repo-level work (e.g. linting, build, tests, refactoring, etc.)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
    • I have included test coverage
  • Breaking change (fix or feature that would cause existing functionality/userland code to not work as expected)
    • Explain why a breaking change is necessary:
  • This change requires (or is) a documentation update
    • I have added necessary local documentation (if appropriate)
    • I have added necessary itty.dev documentation (if appropriate)

@kwhitley kwhitley changed the title Feature: baseless nested routers working (appx +50 bytes) Draft: baseless nested routers working (appx +50 bytes) Jan 8, 2024
@kwhitley kwhitley changed the title Draft: baseless nested routers working (appx +50 bytes) DRAFT: baseless nested routers working (appx +50 bytes) Jan 8, 2024
@kwhitley
Copy link
Owner Author

This adds support for a few simple scenarios, while still requiring complex explanations/bypasses for more complex ones. In the end, that is likely not worth the bytes.

@kwhitley kwhitley added the new feature New feature or request label Mar 12, 2024
@kwhitley kwhitley added discussion Let's discuss ON HOLD and removed discussion Let's discuss labels Mar 12, 2024
@kwhitley kwhitley marked this pull request as draft March 22, 2024 17:42
@kwhitley
Copy link
Owner Author

kwhitley commented Mar 22, 2024

Closing this one for now until we are able to achieve the desired effect:

  • in a way that doesn't sacrifice performance (path concat should be done once, rather than at each route test)

  • without a low enough byte count to justify the inclusion

  • in a more universal way than I was able to achieve (e.g. normal route params should work in nested paths)

    router.all('/:collection/*', collectionRouter) // should work without defining base path in child router

    In the meantime, I was (sadly) unable to satisfactorily check all of those issues in the same attempt.

@kwhitley kwhitley closed this Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request ON HOLD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant