-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Loading a partial config into an array doesn't populate default values #27
Comments
* Actually add/remove extensions from `Config.Extensions.extensions` * Fix the file path where `ExtensionConfigTemplateLoader` looks for extensions * Prevent the first `loadSync()` call from creating a `config.json` file Works around bpatrik/typeconfig#27
That is work as intended. Lat say you have a default config with an array (like list of link), but the user wants to make that list empty. The lib will save an empty array to json and on the next load restore it.
Adding new fields does not really supported dynamically. The lib was built with the intention of knowing the structure of the config in compile time. That said, I added a genericconfig type class that support mutation at some extent dynamically, but its not feature complete yet.
That might be ok. I wanted the config to be able to load partial configs: the li also parses cli, ENV and url query params and handles them the same way as it handles json file inputs. So loading from partial config should remove the undefined part of the config. In contrary there should be a way to set empty array.
i can understand that, but hashes are not really supported, or at least not fully implemented. Best would be creating a ConfigHash class that has some logic in it and through error on using plain js maps.
I made some changes in pigallery to overcome some of the issues. In bpatrik/pigallery2#847, I think the issue is not that an empty array overrides avaialble extensions. Extensions are dynamically loaded and should have been populated again after loading the config from file. |
I was using the term "hash" in a general way to mean "a structure that supports key/value pairs". But you're right, these configs are actually classes with member variables defined at compile time, not true hashes that support dynamically adding and removing key/value pairs. |
This is related to pull request #26. I thought it was best to demonstrate the issue with a concrete test instead of describing it conceptually.
Basically, if you create the config classes with an array, then dynamically add SubConfigClass instances into the array, loading a config file with an empty array will produce a config with an empty array.
This behavior is analogous to when a config class adds a new field (or a subConfig as a field), then loads a config file that doesn't have any values for those fields. In pull request #25, this corresponds to the "should load from partial config" and "should load subconfig from partial config" tests that I added. Unlike with arrays, after the load, the config still includes the missing fields, and they have the default values.
I would expect the behavior to be consistent between hashes and arrays. Either default values for both types should persist after a
load()
call, or they should be deleted if missing from the config file. Having different behaviors for hashes and arrays is confusing.Why is this a problem? In bpatrik/pigallery2#847, one of the problems is that when an extension is added to an already-established site, the config doesn't show up in the admin page. This happens because the extension configs are added as elements in an array. Even though the extension config class instance is dynamically added to the config array, it is removed once a config file is loaded that doesn't have that array entry.
The text was updated successfully, but these errors were encountered: