Open
Conversation
Author
|
@dotnet-policy-service agree |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a new Razor directive,
@keyedinject, to support keyed dependency injection in Razor views and components.The directive is intentionally defined with a fixed token shape where the key is a required string literal placed at the end of the directive.
This approach is backward compatible and avoids changes to Razor’s parsing or directive grammar.
Overview
I initially made this out of curiosity after reviewing #9286. For backwards compatibility, avoiding parser changes, keeping with existing conventions, simplicity and not opening up bigger discussions I have gone with an approach of making a new directive for keyed DI injections and having the key as a non-optional string token on the end of the directive (as the key is kind of like a dictionary key).
This was for the following reasons:
Directives do not support optional preceding tokens. Adding them in is not a trivial task and would require significant changes to how razor/cshtml is parsed. Supporting optional preceding tokens would require non-trivial changes to the Razor parser, directives etc. which is outside the scope of this change.
Semicolons. The inject attribute supports semi-colons. From what I could see unless I did something very hacky or re-wrote how razor/cshtml parsed then having an optional string token means something could get scooped up as the third attribute. Doing either of those things is a bit unnecessary and beyond the scope of the issue.
The key is intentionally placed last along with using a separate
@keyedinjectdirective identifier to avoid ambiguous parsing and malformed directive cascades when the key is omitted.Keyed injection has conceptual distinction from regular injection.
A keyed service must always be resolved using a key. Representing this via a separate directive makes the behavior explicit.
The other proposals were not aligned with existing directive patterns. They were really just member declarations in razor.
A preceding key was also suggested.
In addition to the syntax/parsing issues mentioned above, this seemed akin to accessing a dictionary by writing
[“someKey”]someDictionaryor passing an argument with a value in a CLI byssh 2222 -p. To me it seems you go in almost every case<Type of Thing>then<Selector>. This may be better served by a@member/@classmemberdirective which seems beyond the scope of the issue/feature.The
<Type> <MemberName> <Key>syntax mirrors existing directive usage and avoids introducing member-declaration-style syntax, which is not idiomatic for Razor directives.Risks
This adds the feature without breaking backward compatibility or opening up unnecessary complexity. A possible downside is if more directive syntax options are introduced later, but the code should be easy to consolidate if that happens.
Non feature related changes
end ✌
This is my first pull request to this repo so I hope this is not a bother - feedback is welcome. I tried my best to stay within the coding style/guidelines that I found in the code base and to make the least disruptive changes. I hope it is welcome.