-
Notifications
You must be signed in to change notification settings - Fork 631
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
Investigate issue with TimeTagHelper #2089
Comments
Tried to have a look at this and it appears to be a problem with Autofac and more precisely: containerBuilder.RegisterAssemblyTypes(typeof(Startup).GetTypeInfo().Assembly).AsImplementedInterfaces(); To test this create a new Asp.net core 2.0 web app. Create the tag helper and register it. Then add the following code to the end of ConfigureServices in Startup.cs:
If the containerBuilder.RegisterAssemblyTypes... row is commented out it will work, otherwise crash. I haven't been able to figure out why this causes the app to crash yet, but will try to investigate further. |
Finally had some time to look closer at this. The problem comes from a new feature (bug?) in ASP.Net Core 2.0 where you can register a TagHelper with dependency injection to be injected to the head or body of a page. Microsoft does this with the application insight javascript code. More information here aspnet/Mvc#5728. This causes a problem for us because of this line of code in Services.cs:
This registers every concrete class in the Allready assembly as themselves and this includes TimeTaghelper and TimeZoneNameTagHelper. To solve this we can exclude classes implementing TagHelper from being injected by changing the code to:
|
Thanks for tracking this down @mk0sojo. I figured it was something to do with the change in 2.0 around the head injection, but for the life of me couldn't work out why. Hadn't considered this global DI registration even existed. I'll take your PR for a spin locally but this sounds like a good fix. I'm also curious to expand on why we need to register everything this way. I prefer more controlled and explicit registrations which we might be able to review. |
Yeah...I think this just dates back to perhaps what was available when the project was conceived. Remember that the roots here predate most IoC containers coming over to Core, much less being mature. This is solved in PR #2198 so we can close this out. Thanks folks! |
After 2.0.0 upgrade the TimeTagHelper was presenting strange issues and appeared to be invoked for the head and body tags which led to malformed output.
A workaround was put in to prevent this - see https://github.com/HTBox/allReady/blob/master/AllReadyApp/Web-App/AllReady/TagHelpers/TimeTagHelper.cs#L35
We should investigate the root cause and look to fix this. Copying the tag helper code into a clean project it seemed to work correctly, so this appears to be something specific to the allReady code base.
The text was updated successfully, but these errors were encountered: