-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Remove IViewsProvider and make view lookup to the feature provider #5262
Conversation
Partial fix to #4760 |
cc @javiercn |
9109dd5
to
75a1d66
Compare
cc @rynowak |
} | ||
|
||
protected override Type GetViewInfoContainerType(AssemblyPart assemblyPart) | ||
=> _containerLookup[assemblyPart]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Do we put the arrow on a new line? It kinda looks weird 😵
LGTM |
Nice! LGTM. Is there another issue to make views work generally with application parts? |
@davidfowl - #5144 one. I'll speak to @danroth27 and see if he has a particular design thought for it. We also have to figure out how we'd want to talk about embedded views versus precompiled views (and when we'd want to prefer one over the other). |
@pranavkm @davidfowl I would think that precompiled views supersede embedded views. Even if you had some sort of overriding mechanism in which you could drop in a file and pick that instead of the existing view, I don't see any advantage for anyone creating a library with views to choose embedding them vs precompiling them. (I would assume you would package both dlls (library + precompiled views) in the nuget package and get them into your app when you install the package. |
Using precompiled views with libraries has a couple of issues that I think still need to be thought through. One is that the project has to target netcoreapp1.0 in order to be able to precompile the views, which seems like a hack. Another is figuring out how pack will work when you have precompiled views. With embedded views I don't think you have these problems. |
That said, I agree that we don't want the support burden of two solutions that pretty much solve the same scenario. I lean towards closing #5144 and focusing on solving the remaining precompilation issues. |
No description provided.