Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Remove IViewsProvider and make view lookup to the feature provider #5262

Closed
wants to merge 1 commit into from

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Sep 9, 2016

No description provided.

@pranavkm
Copy link
Contributor Author

pranavkm commented Sep 9, 2016

Partial fix to #4760

@pranavkm
Copy link
Contributor Author

pranavkm commented Sep 9, 2016

cc @javiercn

@pranavkm pranavkm force-pushed the prkrishn/mvcviewsprovider branch from 9109dd5 to 75a1d66 Compare September 9, 2016 18:56
@pranavkm
Copy link
Contributor Author

pranavkm commented Sep 9, 2016

cc @rynowak

}

protected override Type GetViewInfoContainerType(AssemblyPart assemblyPart)
=> _containerLookup[assemblyPart];
Copy link
Member

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 😵

@javiercn
Copy link
Member

javiercn commented Sep 9, 2016

LGTM :shipit: when @rynowak is happy

@davidfowl
Copy link
Member

Nice! LGTM. Is there another issue to make views work generally with application parts?

@pranavkm
Copy link
Contributor Author

@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).

@danroth27
Copy link
Member

I wasn't envisioning any particular design for #5144. Isn't making views work generally with application parts goodness regardless of whether we do #5144?

@javiercn
Copy link
Member

@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.

@danroth27
Copy link
Member

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.

@danroth27
Copy link
Member

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.

@pranavkm pranavkm closed this Sep 14, 2016
@pranavkm pranavkm deleted the prkrishn/mvcviewsprovider branch September 14, 2016 20:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants