PathBase support for React UI #511
Conversation
Uses in memory for all js files and index.html Some code cleanup is still neccessary but it works frm first testings For all other files that are not js files and not the index html PhysicalFileInfo is taken as fallback
Fixed Logger null error
…ptions.cs Co-Authored-By: Loïc Sharma <sharma.loic@gmail.com>
…ly in production mode commented reactdevserver back in whoch will be used in dev mode
…to basePathSupport
|
This looks great! Thank you for starting this. I left some comments but I need to dig deeper as I don't fully understand the static file providers just yet. |
| return new DirectoryContents(this, new DirectoryInfo(Path.Combine(_root.FullName, subpath.TrimStart('/')))); | ||
| } | ||
|
|
||
| public IFileInfo GetFileInfo(string subpath) |
There was a problem hiding this comment.
I'd prefer it if this feature was a little more "pay for play". Right now we always cache files in memory even the PathBase configuration wasn't set. It'd be better if we don't cache or replace __BAGET_PLACEHOLDER_... if PathBase isn't configured.
There was a problem hiding this comment.
I know three options to implement a custom pathbase with ASP.NET and Node:
- Replace
__BAGET_PLACEHOLDER_...in every affected file and cache them. I would not expect any performance penalty as files are kept in memory. An instance of BaGet occupies ~100 MB RAM and caching all JavaScript files would another ~700 KB. - Replace
__BAGET_PLACEHOLDER_...only inindex.htmland implement code inconfig.tsxto dynamically replace__BAGET_PLACEHOLDER_...with an empty string. This would reduce the additional memory usage for standard deployments to less than 3 KB. - Omit placeholders in
index.htmland use the concept from 2. for JavaScript. If a pathbase is configured we could editindex.htmlwith regular expressions. This would require a full test coverage because we would have to rely on implementation details of the Node compiler.
If you know a better way to implement a custom pathbase please describe it to me or tell me which of these three options would like to have in BaGet.
There was a problem hiding this comment.
@loic-sharma What's your opinion on that?
|
I think we can simplify |
Co-authored-by: Loïc Sharma <sharma.loic@gmail.com>
Yes, we can replace However, using the default |
| //if (env.IsDevelopment()) | ||
| //{ | ||
| // spa.UseReactDevelopmentServer(npmScript: "start"); | ||
| //} |
There was a problem hiding this comment.
This React development server is really a headache for me. If it is enabled in all development environments, integration tests for my SPA file provider with custom path base fail. In addition to that we have a second middleware for serving SPA static files which should be disabled when using the React server.
My idea for a workaround would be to add a setting BaGetOptions.DisableReactDevelopmentServer. Per default, all development environments would use the React server and unit tests would disabled it with an in-memory configuration. Any other ideas?
|
This repository has been inactive for too long to keep this PR up to date. There are better ways to implement a custom base path for Vue.js which I found out working on another project. I am sure such an approach would work for BaGet and React, too. |
Summary of the changes
ISpaStaticFileProviderto replace these placeholdersAddresses #276
Fixes #504
This is an improved version of #388 by @szwenni
ConfigureHttpServices