Skip to content

https://github.com/umbraco/Umbraco-CMS/issues/7469 - Fixed Ambiguous… #7470

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

Merged
merged 2 commits into from
Jan 17, 2020

Conversation

bergmania
Copy link
Member

@bergmania bergmania commented Jan 17, 2020

Fixes #7469

  • Renames our extension method from Value to ValueByExpression
    • There really is no good solution to this - An alternative is to remove the extension method from the embedded modelsbuilder

@clausjensen clausjensen merged commit df1e006 into v8/8.5 Jan 17, 2020
@clausjensen clausjensen deleted the v8/hotfix/7469-ambiguous-extension-method branch January 17, 2020 13:32
@sniffdk
Copy link
Contributor

sniffdk commented Jan 17, 2020

Perhaps it could be named .Value and .ValueFor
This would put the naming in line with how mvc already does it eg. Html.Label and Html.LabelFor

@Shazwazza
Copy link
Contributor

I'm pretty sure we can solve this with AppDomain events, also considering that Umbraco.ModelsBuilder.Embedded is a separate assembly may make it easier. I'll test some things out.

@ronaldbarendse
Copy link
Contributor

Having the same namespace (and methods) in different assemblies will inevitably cause these kind of problems, but instead of changing the method, you should use extern alias (https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/extern-alias) or better yet: rename the namespace 😉

@Shazwazza Isn't this a compiler problem, so how would AppDomain events help prevent this?

@Shazwazza
Copy link
Contributor

@ronaldbarendse yes you are right but the immediate issue is that of dynamic compilation in razor and app_code. I pushed a temp fix here for razor compilation work arounds #7475 but there isn't a possible work around for app_code. These were just interim hotfix's, we will be pushing a change to the Umbraco.ModelsBuilder package in the next release that won't have the overlap/duplication of the code in the CMS which will solve the problem. Namespace rename isn't a solution either because these APIs are used in razor views mostly and to have a seamless upgrade experience both namespaces would need to be imported and we'd have the same problem.

@ronaldbarendse
Copy link
Contributor

Hmm, seems releasing new versions of both Umbraco and ModelsBuilder is still the only real solution to this problem (as you mentioned in the linked PR)... That will break the supported versions of ModelsBuilder though (@zpqrtbnk be sure to make this at least a new minor release, so older Umbraco versions can still get patch updates).

Why wasn't the full ModelsBuilder package transferred to Umbraco? It was already renamed from Zbu.ModelsBuilder, included as core package and it's even MIT licensed... API support and the VS extension could then still be supported by Stephan (possibly as paid extensions if that's required to keep it updated).

@Shazwazza
Copy link
Contributor

If people are using this ext method in compiled code and we release a new MB version, yes they will need to re-compile but they will need to do that now anyways with this conundrum. The new Umbraco.ModelsBuilder package will target umbraco 8.5.3. There's a blog post about what parts of MB were transfered here https://umbraco.com/blog/umbraco-85-release/

This next Umbraco.ModelsBuilder release to solve this single namespace /ext issue will most likely be the last MB release under this package name, but I guess we'll see what transpires with the community package.

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

Successfully merging this pull request may close these issues.

None yet

5 participants