feat/Admin UI local storage adapter + build improvements#345
feat/Admin UI local storage adapter + build improvements#345jonaspm wants to merge 7 commits intobknd-io:mainfrom
Conversation
- Move local adapter schema and type to dedicated module - Add StorageLocalAdapterBase for schema-only usage without Node.js deps - Update StorageLocalAdapter to extend the new base class - Register StorageLocalAdapterBase in media registry - Re-export schema and base class from local adapter index
- Move all adapter schemas to a new adapter-schemas.ts with no Node.js deps - Remove StorageLocalAdapterBase and related files - Update registry to only register adapters safe for all environments - Dynamically import and register local adapter at runtime in Node.js/Bun - Update AppMedia to use new getAdapterClass logic - Update media schema builder to use adapterSchemas - Comment out AdapterIcon in UI to avoid missing icon for local adapter
Simplify and centralize externalization of Node.js and Bun built-ins. Remove redundant esbuildOptions and external config in build steps.
jonaspm
changed the title
cameronapak
left a comment
cameronapak
left a comment
There was a problem hiding this comment.
I'm not someone who can truly affirm and validate this code since I'm not Dennis. However, I think the code feels good.
question: Can make it where the local adapter is available if the mode-specific bknd config flag of isProduction is false? (see https://docs.bknd.io/usage/introduction/#mode-helpers)
There was a problem hiding this comment.
This seems like a good idea. I like the abstraction and keeping the schemas in a specific place.
There was a problem hiding this comment.
Yeah this change was a consequence of an error that was showing up when loading the Local Storage Adapter into the UI, the Node imports blocked me from just loading it into the client, therefore I had to separate the schema of the class.
There was a problem hiding this comment.
Build improvements come from trying to build and test on Windows and errors showing up.
There was a problem hiding this comment.
Automatic path creation for the local storage adapter was not needed really but was easy to implement and is a nice to have when you want to save time by avoiding having to configure the path manually.
|
And btw, I love the idea of this feature in the Admin Dashboard UI and hope it gets approved |
In code mode it is pretty easy to control what lands into the bknd config before it gets loaded but in the UI mode it's not. I think it could be implemented an environment specific config, let me explain: You start setting up production configuration, that is the main source of truth, then you decide that you want a different configuration for development environment, therefore you go into the UI, select "Development" environment at the top navbar then any deviating config is stored into that mode, so now you have main complete production config + development only overrides. does that make sense? Gemini Nano Banana almost nailed the idea: |
|
That makes sense. It's like how on Supabase you can do branching to have a staging environment and config and a separate one for production What do you think the config would look like in code mode to handle allowing for both? Or would there be multiple bknd configs? |
In code I think it could be like this: the config can be changed just by having multiple json config files. let's say you are using the appconfig-dev.json and the app is running smoothly then, you change the code to import the appconfig-prod.json, then the application reloads and the new configuration is respected. |
|
hello @dswbx @cameronapak is there anything else needed from my side to get these changes merged? best regards! |
|
I'll admit I've been behind on things due to a pickup of focus at work. I cannot answer your question at this second. But when I get a stretch of free time to look through this, I'll make sure that I'm present and can respond well. From what I remember, it'd be nice if this could only show local if it's not production in the config, but maybe I'm oversimplifying it. My main concern that I have is we should not have local in production unless the user's running on their own VPS or something, right? |
If the environment does not support local storage then it will gracefully fail when required node packages are dynamically imported. I think we should let the user choose local storage even if it is production for the ones who's use case require it. |
2 participants
This pull request refactors how media storage adapters and their schemas are managed, with a focus on improving modularity, runtime flexibility, and compatibility across different environments (Node.js, Bun, browser). The key changes include extracting adapter schemas to a central location, supporting dynamic adapter loading, and cleaning up registry usage. These improvements make it easier to add new adapters, avoid unnecessary dependencies, and ensure that UI and runtime environments only load what they need.
bun link bkndAdapter schema and registry refactor:
local,s3,cloudinary) are now defined in a new central fileadapter-schemas.ts, which is free of Node.js dependencies and safe to import in any environment. This enables the Admin UI to display configuration options for all adapters without requiring the actual adapter implementations.MediaAdaptersexport is removed to prevent accidental static imports of Node.js-dependent code. [1] [2]media-schema.tsnow uses the new centralized adapter schemas, ensuring that UI and validation code do not require Node.js modules.Dynamic adapter loading and backwards compatibility:
AppMediamodule now dynamically imports thelocalstorage adapter at runtime if it is not already registered, providing clear error messages if used in unsupported environments. This avoids bundling Node.js code in browser builds and supports flexible adapter registration. [1] [2] [3]StorageLocalAdapterre-exports the schema and type from the new central schema file for backwards compatibility, and ensures parent directories exist when writing files. Minor improvements include more robust error handling and argument naming. [1] [2] [3] [4] [5]Loca storage adapter:
Testing:
AppMedia.spec.tsto document its purpose as a debug helper for logging the media config.UI:
Build Process:
distcleaning command with a cross-platform Node.js implementation, ensuring compatibility with different operating systems. [1] [2]