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

TS/JS Warnings after creating a .NET 8 ASP.NET Core Web App #56471

Closed
1 task done
sayedihashimi opened this issue Jun 26, 2024 · 18 comments
Closed
1 task done

TS/JS Warnings after creating a .NET 8 ASP.NET Core Web App #56471

sayedihashimi opened this issue Jun 26, 2024 · 18 comments
Assignees
Labels
area-ui-rendering Includes: MVC Views/Pages, Razor Views/Pages feature-templates

Comments

@sayedihashimi
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

REPRO STEPS:
1.File > New > Project > ASP.NET Core Web App (Model-View Controller)>Select .NET 8.0 > Create.
2.Open Views->Shared->_Layout.cshtml file, TypeScript typings will be installed automatically, then observe error list window

image

We have discussed internally and have a few proposals to consider in that thread.

cc @Artak

Expected Behavior

No warnings

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 26, 2024
@v-Lily
Copy link

v-Lily commented Jul 1, 2024

This issue also reproduces on .NET 6.0 ASP.NET Core Web App.

@balachir balachir added this to the 9.0-preview7 milestone Jul 11, 2024
@balachir
Copy link

@mkArtakMSFT fyi, this is the issue tracking changes to 9.0 templates related to TS/JS warnings from bootstrap and jquery-validation dependencies. Internally we were tracking this with AzDO bug 2065646. I went ahead and marked this for 9.0-preview7 milestone for your triage.

@mkArtakMSFT mkArtakMSFT removed this from the 9.0-preview7 milestone Jul 23, 2024
@mkArtakMSFT mkArtakMSFT added area-ui-rendering Includes: MVC Views/Pages, Razor Views/Pages and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Jul 23, 2024
@javiercn
Copy link
Member

@sayedihashimi what's the expectation here, since these warnings come from dependencies and not from code in the user app.

Is there a way we can disable warnings in those situations?

@sayedihashimi
Copy link
Member Author

@sayedihashimi what's the expectation here, since these warnings come from dependencies and not from code in the user app.

Is there a way we can disable warnings in those situations?

@joj can we disable warnings from dependencies?

@joj
Copy link

joj commented Jul 24, 2024

Yep. The template needs to have this set in the csproj:

<TypeScriptSkipLibCheck>false</TypeScriptSkipLibCheck>

That should prevent type checking on "lib" files.

@sayedihashimi
Copy link
Member Author

sayedihashimi commented Jul 24, 2024

@javiercn what's the best way to get the property <TypeScriptSkipLibCheck>false</TypeScriptSkipLibCheck> to these projects? Should we add it to the web sdk so it applies to all projects as a default value, then users can override in the project file?

Or @joj shouldn't we default that to false and have users override that to true? I think most people don't want warnings/errors coming from deps.

@joj
Copy link

joj commented Jul 24, 2024

Agreed on that, but also we don't see this request as often as one would thing it should happen, so I'm thinking there must be something else there. I honestly haven't tried that behavior in a while (specially in an ASP.Net Core context). My assumption (and I may be wrong on this one) is that if the node_modules are not present in the solution, we will ignore them anyway. The default is to not add that folder to the solution. Again, that might have recently changed (either one of those). We need a more long-term fix for sure, but first I would like to fully identify the issue.

@javiercn
Copy link
Member

@sayedihashimi applying the value on the SDK seems reasonable to me.

I assume if people drop a tsconfig.json file that will override?

@joj
Copy link

joj commented Jul 25, 2024

@javiercn yes, tsconfig will always override in VS. We were looking at it a bit more, though, and the files with the warnings are actually part of the project and the warnings are legitimate. I'm not sure if we have a way of excluding specific files (other than node_modules) from type checking but even if we did, I believe we should be fixing those warnings.

@javiercn
Copy link
Member

@joj the files are libraries that are placed under lib in the project wwwroot folder. They are the dist folder of the npm package for bootstrap, jquery, etc. We shouldn't be fixing anything there, just ignoring them.

@sayedihashimi
Copy link
Member Author

@joj the files are libraries that are placed under lib in the project wwwroot folder. They are the dist folder of the npm package for bootstrap, jquery, etc. We shouldn't be fixing anything there, just ignoring them.

Agreed that we should not directly edit those files. Are we sure that those deps are up to date? If not, maybe updating them will fix the warnings. Otherwise we need some way to get those files excluded, I think it's reasonable to exclude the entire wwwroot/lib folder because it's supposed to be for deps and not user code. I understand that users can put code there but that's not our guidance.

@joj
Copy link

joj commented Jul 25, 2024

Agreed, but from the language service perspective those are just files. We don't have logic to ignore folders and wwwroot\lib is not a convention that TS knows of (AFAIK). I do believe if people should not be updating those files then they shouldn't show up in the solution. If they didn't, the language service would ignore them. Is that what you mean by "exclude", Sayed?

@sayedihashimi
Copy link
Member Author

@joj removing them from the project template only fixes this for new projects that get created. I think we need to find a way to treat wwwroot/lib in aspnet core projects like node_modules.

@mkArtakMSFT
Copy link
Member

I think we need to find a way to treat wwwroot/lib in aspnet core projects like node_modules.

+1 to this. I don't think we should touch the template.

@mkArtakMSFT
Copy link
Member

@sayedihashimi I'm going to let you drive this work then (either in SDK or on the VS side whichever you choose).

@joj
Copy link

joj commented Jul 25, 2024

I don't think that's the right choice. That will only hide very legitimate warnings. We can find ways of excluding files in the language service via configuration (that will still go on the template), but in this case I believe the right solution is to fix the warnings.

@sayedihashimi
Copy link
Member Author

I don't think that's the right choice. That will only hide very legitimate warnings. We can find ways of excluding files in the language service via configuration (that will still go on the template), but in this case I believe the right solution is to fix the warnings.

I think the best we can do is to update the deps to the latest. We can't modify the files ourselves directly. Can we get the work started to get this configured via templates/SDK? We don't have much time if we need to get the aspnet templates/SDK updated.

@sayedihashimi
Copy link
Member Author

This no longer repros with the latest preview builds of 17.12, I will close this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ui-rendering Includes: MVC Views/Pages, Razor Views/Pages feature-templates
Projects
None yet
Development

No branches or pull requests

6 participants