Skip to content

Conversation

@HaoK
Copy link
Member

@HaoK HaoK commented Jun 11, 2021

Part of #29597

Moving to helix merged the websites so there are additional css/js files, more tightly scoping the globs avoids the additional js/css files

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 11, 2021
@HaoK HaoK requested a review from a team June 11, 2021 14:38
@HaoK HaoK marked this pull request as ready for review June 11, 2021 14:38
@HaoK HaoK requested review from javiercn and pranavkm as code owners June 11, 2021 14:38

<!-- Globbed script tag with existing files and version -->
<script asp-src-include="**/*.js" asp-append-version="true">
<script asp-src-include="**/styles/**/*.js" asp-append-version="true">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually put JavaScript under the styles directory? Seems weird...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really have any more context than you, but this is the expected styles directory https://github.com/dotnet/aspnetcore/tree/main/src/Mvc/test/WebSites/HtmlGenerationWebSite/wwwroot/styles, the issues it that all the test websites get their wwwroots combined when I did the helix conversion, since they all got squashed together, so **/*.js finds a bunch of extra files. I don't really have a simple fix for that, so I just updated it to look at the styles directory 🤷

@HaoK
Copy link
Member Author

HaoK commented Jun 14, 2021

Updated to fix globbing taghelper test which had similar issues with script wildcards (fix was explicitly excluding the additional unexpected libraries from the test websites)

@HaoK
Copy link
Member Author

HaoK commented Jul 21, 2021

I'm leaving taghelpers out of this PR to get this in, @captainsafia does this look reasonable to merge as is? The jist of the diffs is all the script files for all the functional test websites show up now due to helix publishing, so these tests with wildcards need to be hand tweaked to exclude the unexpected files

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.

Are we tracking re-enabling the TagHelpers tests somewhere?

@HaoK
Copy link
Member Author

HaoK commented Jul 21, 2021

Yeah I'll leave #29597 open and just update it to say taghelpers are left

@HaoK HaoK merged commit c684217 into main Jul 21, 2021
@HaoK HaoK deleted the haok/mvcf branch July 21, 2021 23:01
@ghost ghost added this to the 6.0-rc1 milestone Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants