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

WIP: Enhance inject route utilities #450

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Dafnik
Copy link
Contributor

@Dafnik Dafnik commented Jul 15, 2024

Adds the following options to all utilities and therefore aligns them.

  • injectParams
    • transform
    • defaultValue
    • injector
  • injectQueryParams
    • defaultValue
    • injector
  • injectQueryParams.array
    • defaultValue
    • injector
  • injectRouteData
    • defaultValue
    • injector
  • injectRouteFragment
    • defaultValue

I also took the chance to rename initialValue to defaultValue (as I think it's a more fitting name).
injectQueryParams supports both values, as to not introduce a breaking change.

Tests are still missing.

@nartc
Copy link
Collaborator

nartc commented Aug 2, 2024

is this still WIP?

@Dafnik
Copy link
Contributor Author

Dafnik commented Aug 3, 2024

Yes, docs and tests are missing.

@Dafnik Dafnik force-pushed the refactor/inject-route branch from 0c240e0 to 231fb26 Compare September 15, 2024 21:46
@eneajaho
Copy link
Collaborator

eneajaho commented Oct 1, 2024

Hello @Dafnik
Thanks for this great PR.

Regarding

I also took the chance to rename initialValue to defaultValue (as I think it's a more fitting name).
injectQueryParams supports both values, as to not introduce a breaking change.

I think initialValue is a better fit there because after the initialValue, the params can change however the router needs, and we shouldn't call it default value (as that would mean that we will reapply it again based on some condition, which we don't because we only need it the first time).

@Dafnik
Copy link
Contributor Author

Dafnik commented Oct 3, 2024

Hi, thank you very much :)

But it is actually a default value. 😅

Lets say we have the route test and use the URL /test?a=1234.
If we use injectQueryParams('a', {defaultValue: '1111'}) we will get the result 1234.
But now the route changes to /test. The query param is null and the default value 1111 will be emitted.

Or did I get this wrong?

@Dafnik
Copy link
Contributor Author

Dafnik commented Nov 11, 2024

Ping @eneajaho

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.

3 participants