-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Add runtime support for Blazor attribute splatting #10357
Merged
Merged
Conversation
This file contains 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
rynowak
commented
May 17, 2019
rynowak
force-pushed
the
rynowak/splatsville
branch
from
May 19, 2019 18:55
1b2fb76
to
7dd8caf
Compare
@SteveSandersonMS @javiercn @pranavkm - ready for your review. I need to merge the |
rynowak
commented
May 19, 2019
rynowak
commented
May 19, 2019
rynowak
commented
May 19, 2019
rynowak
commented
May 19, 2019
rynowak
commented
May 19, 2019
rynowak
commented
May 19, 2019
src/Components/test/testassets/BasicTestApp/DuplicateAttributesComponent.razor
Outdated
Show resolved
Hide resolved
pranavkm
reviewed
May 20, 2019
src/Components/Analyzers/test/ComponentCaptureExtraAttributesParameterMustBeUniqueTest.cs
Outdated
Show resolved
Hide resolved
rynowak
force-pushed
the
rynowak/splatsville
branch
4 times, most recently
from
May 26, 2019 01:14
51fdeb6
to
8489a49
Compare
Looking at it. |
pranavkm
approved these changes
May 28, 2019
- Adds AddMultipleAttributes - Fix RTB to de-dupe attributes - Fix RTB behaviour with boxed EventCallback (#8336) - Add lots of tests for new RTB behaviour and EventCallback
This was being flaky while I was running E2E tests locally, and it wasn't using a resiliant equality comparison.
Adds the option to mark a parameter as an *extra* parameter. Why is this on ParameterAttribute and not a new type? It makes sense to make it a modifier on Parameter so you can use it both ways (explicitly set it, or allow it to collect *extras*). Added unit tests and validations for interacting with the new setting.
This is the *easy way* to write an analyzer that looks at declarations. The information that's avaialable from symbols is much more high level than syntax. Much of what's in this code today is needed to reverse engineer what the compiler does already. If you use symbols you get to benefit from all of that. Also added validation for cascading parameters to the analyzer that I think was just missing due to oversight. The overall design pattern here is what I've been converging on for the ASP.NET Core analyzers as a whole, and it seems to scale really well.
This involved a refactor to run the analyzer per-type instead of per-property.
rynowak
force-pushed
the
rynowak/splatsville
branch
from
May 28, 2019 19:19
854f6c3
to
3c9425f
Compare
rynowak
force-pushed
the
rynowak/splatsville
branch
from
May 28, 2019 19:38
3c9425f
to
95832c6
Compare
ghost
deleted the
rynowak/splatsville
branch
May 29, 2019 00:17
Breaks on unixy style OSs I think its the embedded crlf in the test string; have added fix #10610 |
This was referenced May 29, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Implements: #5071
See design notes here for details: #5071 (comment)
Opening a PR now to get early feedback and discussions. This will require checkins and updates from the tooling repo to fix the runtime behaviour of markup blocks and to add support for splatting as a language features (syntax not yet locked).