Fix for change in .NET 6 for context.Request.Path.Value

[tchadwick]

In .NET 6 the value of context.Request.Path.Value appears to always have a leading "/" regardless of the reference in the html file. An example of this is when you add a javascript bundle like: pipeline.AddJavaScriptBundle("Scripts/libraries.js", "Scripts/jquery.gridster.js"); and reference it in the html file like: <script src="Scripts/libraries.js"></script> The bundle is added as an asset with the value of "Scripts/libraries.js" but the value of context.Request.Path.Value will be "/Scripts/libraries.js" resulting in no match. This proposed change will sanitize all assets to have a leading "/" to ensure this matches.
In my companies use of multiple javascript bundles with less compilation, this worked for us, but I'm not sure if this will mess with/break other possible routes. This needs someone who is more familiar with the entirety of the WebOptimizer solution to ensure it doesn't introduce regressions for other people. (I also tested it in older versions of .NET and it appeared to work without issue) I see the failing tests, but it's because the asset being added is getting the leading "/" perhaps we need handle this by removing it at the AssetMiddleware.cs's: string path = context.Request.Path.Value; But that doesn't seem to be the right approach either. Hopefully @madskristensen or one of the others can take a look and evaluate the regression possibilities of this one.

[madskristensen]

The unit tests failed on this PR

[tchadwick]

The unit tests failed on this PR Hey @madskristensen, I was hoping you would have more insight to the right approach here. I'm worried about side-effects of putting the leading "/" on every asset. As I mentioned in my previous comment, my testing seems to validate that it works both on old .NET versions and the latest, but it seems to be a change that could have side-effects for the broader library. If you think it's fine, I'll update the tests to have them test with this new behavior in mind, but I wanted to make sure this is the direction we take with this PR. Thoughts?

[madskristensen]

That sounds like a breaking change.

[gumbarros]

This proposed change will sanitize all assets to have a leading "/" to ensure this matches.

Maybe a if statement to .NET 6 will be necessary to the new included lines? With this If statement this will not be a breaking change to .NET Core 3.1.

[tchadwick]

This proposed change will sanitize all assets to have a leading "/" to ensure this matches.

Maybe a if statement to .NET 6 will be necessary to the new included lines? With this If statement this will not be a breaking change to .NET Core 3.1.

I tested it for our situation (bundling, minification, globbing, and less compilation) in .NET Core 3.1 and it worked as well. What I'm doing is sanitizing the matching of all asset routes to always have a leading "/" when added, then normalizing all requests so that if they didn't have a leading "/" it would add one to match. I think the only breaking situation would be if someone were to add two assets with the exact same name, one with the leading "/" and one without. This situation would break the proposed solution. Is it something we see developers doing? @madskristensen

[gumbarros]

@tchadwick good afternoon, I forked your PR and added manually to my project, we're really needing using the WebOptimizer on .NET 6.

I'm getting the following error

Using this setup to prevent any error and just test if works. Already tried adding the / before the bundle.

        services.AddWebOptimizer(pipeline =>
        {
            pipeline.MinifyCssFiles("css/jjmasterdata/jjmasterdata.css");
            pipeline.AddCssBundle("css/bundle.css", "css/jjmasterdata/jjmasterdata.css");
        });

[rdorris]

Any plan to accept this PR?

[madskristensen]

Thanks