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

ExpressionHelper contains a compatibility bug with Roslyn compiler #117

Closed
doggy8088 opened this issue Jan 8, 2018 · 7 comments
Closed
Assignees
Labels
3 - Done bug cost: S Will take up to 2 days to complete

Comments

@doggy8088
Copy link
Contributor

doggy8088 commented Jan 8, 2018

There is a bug in the ASP.NET MVC 5.2.3 for a long time. It's hard to explain this in English. So I made a few steps to reproduce this problem.

STEPS TO REPRODUCE THE PROBLEM

  1. Create a simple ASP.NET MVC 5 project using Visual Studio 2017
  2. Apply change to the follow code snippets. Only 3 files to change.
    https://gist.github.com/doggy8088/58dd3c773ef9877155b19ebb2fda8525
  3. Start debugging and navigate to /Home/Index page. Then you will see the result like below:
    image
    As you can see, the @Html.TextBoxFor(m => data[i].Name) generated field name in the for-loop in the view are all wrong.

MORE INFO

This problem is only exists when ASP.NET MVC 5 project using Roslyn to compiler views.

I was wrote an article about 2 years ago. My article is written in Chinse. If some of you can read Chinese, you can also reference my article over here: https://blog.miniasp.com/post/2016/03/06/ASPNET-MVC-5-View-Roslyn-problem-workaround.aspx

I actually proposed a pull request to fix this problem about 2 years ago too. You may check it out too.

@mkArtakMSFT
Copy link
Member

@dougbu, can you please look into this?

@mkArtakMSFT
Copy link
Member

Thanks for contacting us. One of our team members will look into it and get back to you.

@IanKemp
Copy link

IanKemp commented Feb 5, 2018

tl;dr backporting the fixes from this commit in ASP.NET Core MVC should resolve the issue.

@doggy8088
Copy link
Contributor Author

@mkArtakMSFT Any new progress on this issue?

@mkArtakMSFT
Copy link
Member

@dougbu, did you get a chance to look into this?

@dougbu
Copy link
Member

dougbu commented Feb 5, 2018

@mkArtakMSFT I read the comments over the weekend and no longer think we need an investigation.

We fixed a related problem in ASP.NET Core and can do something similar here. That should fix the problem. Should however spend some time adding a number of tests covering more-complicated lambda expressions in this repo.

BTW, I'm surprised Roslyn versions are material here: I would expect the set of captured local variables to remain fairly consistent.

@mkArtakMSFT mkArtakMSFT added this to the 3.2.5 milestone Feb 5, 2018
@dougbu dougbu added the cost: S Will take up to 2 days to complete label Mar 3, 2018
dougbu added a commit that referenced this issue Mar 12, 2018
@dougbu
Copy link
Member

dougbu commented Mar 13, 2018

b32ab42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Done bug cost: S Will take up to 2 days to complete
Projects
None yet
Development

No branches or pull requests

4 participants