Eptagone / Vite.AspNetCore

Small library to integrate Vite into ASP.NET projects
MIT License
264 stars 35 forks source link

Dev middleware configuration should be optional #20

Closed matteocontrini closed 1 year ago

matteocontrini commented 1 year ago

Hi, I've noticed that when using the development server unless you put some configuration in appsettings.json an exception is thrown:

      System.NullReferenceException: Object reference not set to an instance of an object.
         at Vite.AspNetCore.Services.ViteDevMiddleware..ctor(ILogger`1 logger, IConfiguration configuration, IWebHostEnvironment environment, IHostApplicationLifetime lifetime)
         at InvokeStub_ViteDevMiddleware..ctor(Object, Object, IntPtr*)
         at System.Reflection.ConstructorInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
         at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
         at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
         at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitRootCache(ServiceCallSite callSite, RuntimeResolverContext context)
         at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
         at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite callSite, ServiceProviderEngineScope scope)
         at Microsoft.Extensions.DependencyInjection.ServiceProvider.CreateServiceAccessor(Type serviceType)
         at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
         at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope)
         at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
         at Microsoft.AspNetCore.Http.MiddlewareFactory.Create(Type middlewareType)
         at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext()
      --- End of stack trace from previous location ---
         at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

This is because the dev middleware expects the options object to always be non-null:

https://github.com/Eptagone/Vite.AspNetCore/blob/e1445995e0f427037775f693ced09b03cda8c3f8/src/Vite.AspNetCore/Services/ViteDevMiddleware.cs#L49-L53

However the readme says that:

By default, the manifest name is manifest.json and it's expected to be in the web root folder.

So I guess the aim was to have some defaults that don't require adding configuration manually.

A workaround is to add empty configuration objects in appsettings.json, like:

{
  "Vite": {
    "Server": {
    }
  }
}

Which isn't really ideal.

I would suggest to either make configuration "optional" (with defaults) or to make it clearer in the docs that you always need to add some settings.

Thanks!

Eptagone commented 1 year ago

Hi, you're right. All the options should be optional. I'll fix it, thanks.

Eptagone commented 1 year ago

Done. Fixed with 7ab556bc686c06634c2d1e97f3c2bd2e479cf37f!

matteocontrini commented 1 year ago

Thanks for the quick feedback!

I think there's the same problem in ViteManifest too though.

Eptagone commented 1 year ago

in the json too?

matteocontrini commented 1 year ago

What do you mean? I mean that the configuration is assumed to be always present here too, in the ViteManifest class:

https://github.com/Eptagone/Vite.AspNetCore/blob/2f3dfbdbd671d263742e7dc416f75455fd806535/src/Vite.AspNetCore/Services/ViteManifest.cs#L43

Eptagone commented 1 year ago

Ohh it’s true. I’ll fix it too