Skip to content

Conversation

@Shane32
Copy link
Member

@Shane32 Shane32 commented Jul 12, 2022

I think the UI middleware extension methods should require the path first and then the options, rather than the other way around. I believe this just makes logical sense that you would define the endpoint address and then configure it. Plus it matches the argument ordering of the UseGraphQL method (both now and previously).

Also I generally find that it works best when optional method arguments are additive to existing/required arguments, and this change fixes that issue as well.

This will require a note in the migration doc ( #854 ) if it is merged.

Some other options:

  • Leave the old methods, deprecated
  • Deprecate the path-only syntax leaving only the (options, path) syntax
  • Change the options argument to a builder delegate (if we do this then I would deprecate the old ones rather than removing them)

Since we have a large number of changes going into v7, perhaps now is a good time to make this change.

@Shane32 Shane32 added this to the v7 milestone Jul 12, 2022
@Shane32 Shane32 requested a review from sungam3r July 12, 2022 05:24
@Shane32 Shane32 self-assigned this Jul 12, 2022
Copy link
Member

@sungam3r sungam3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I was thinking about it.

@sungam3r sungam3r added the code style Pull request that applies code style rules label Jul 12, 2022
@Shane32 Shane32 merged commit 839d756 into develop Jul 12, 2022
@Shane32 Shane32 deleted the ui_extension_methods branch July 12, 2022 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code style Pull request that applies code style rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants