Skip to content
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

"Known Attributes" Should only be Ignored in XML, but Nowhere Else #346

Open
NightOwl888 opened this issue Aug 9, 2014 · 3 comments
Open

Comments

@NightOwl888
Copy link
Collaborator

This issue was originally posted on StackOverflow and the complete details are there.

Basically, since custom attributes are now separate entities than properties on SiteMapNode, there is no reason to have a limitation on which names cannot be used for anywhere other than in XML. There is no reason to throw an exception in any case, but the known attributes should be ignored when configuring XML. All other node types should not have a restriction on what names can be used.

@Jogai
Copy link

Jogai commented Apr 15, 2015

In the referenced issue you stated: "I plan to fix this in the next minor version, which could be some time off. In the meantime, you can use the workaround here or don't use any names in your SiteMap that are reserved names."
Wasnt this done in 4.6.18? Or only in the MVC5 variant?

@NightOwl888
Copy link
Collaborator Author

4.6.18 was a patch version. We are trying to respect semantic versioning as much as possible, but the reality is this fix will require changes that will break existing external DI configurations.

My plan for the next minor version (4.7) is to fix #343 by providing an alternative to using external DI by making a new fluent configuration API, similar to what is described in the posts DI Friendly Framework and DI Friendly Library. The goal of the fluent configuration API will be to allow you to supply a replacement for a single interface without having to supply an entire configuration for every interface (as is the case with external DI today). It will still be DI-friendly, but will also be friendly to those who don't use DI or don't want to use it. It will also be a complete replacement for everything that is configured in web.config now. So everything - the configuration options, custom implementations, and even the node configuration, can be done in one unified API (but will still support the existing configuration options).

That said, it will require quite a bit of work to design and implement and my current schedule doesn't allow for much time to maintain MvcSiteMapProvider. I may push forward a 4.7 release that doesn't contain the new API to fix this issue and #310, but I was hoping to release the API as well so when people migrate from external DI to fluent API they will no longer be exposed to breaking changes that don't even apply to their application.

This is more of a "don't do that" bug. It seems silly to break everyone's DI configuration just to fix something that can be avoided altogether by staying clear of reserved names. The same goes for #310 - the simple solution there is to always set the CacheDuration > 0. Both problems also have an alternative workaround that can be applied with external DI if the simple solution isn't good enough. So I consider both of these issues to be of lower priority than providing an alternative to external DI everyone can benefit from.

@Jogai
Copy link

Jogai commented Apr 16, 2015

Thats fine. Just wondering. Will implement DI solution.

Dont hasten things if the quality of the whole project suffers. Fluent api sounds great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants