Commit ec8e4f1
committed
Moved all configuration logic into createConfig.mjs, which validate and returns config. Note:
1. I added config file validation using a combination of
ts-json-schema-generator (to generate a JSON schema from the ConfigFile
type) and ajv (to validate that the config file matches it). This way
we don’t need to include a bunch of custom validation logic for each
config property.
One trade-off here is that SmartDialerConfig isn’t based on the
underlying Go code, so it could fall out of sync.
If we convert all the scripts to TypeScript we can replace this with
e.g. zod or ts-pattern (which I’m familiar with) by starting with a
pattern and deriving both a type and validator function, rather than
starting with the type and deriving a validator function.
2. To simplify things I made a decision to only accept two CLI
arguments: config (for passing in custom config locations) and platform
(to choose build platform without creating redundant platform-specific
configs). This sidesteps the issues with e.g. passing in
additionalDomains (do we accept a comma-separated list?) and
smartDialerConfig (do we accept a JSON string then parse it?).
3. There are still two types: Config and ConfigFile. ConfigFile (used
for config file validation) is based on Config, but with derived
properties (entryDomain, domainList, and smartDialerConfigBase64)
removed, and the remaining properties other than entryUrl optional.
I don’t see any way to avoid this unless we require that the config file
contains all config properties (including the derived properties).
Also note that I renamed these from "BuildConfig*" to "Config*". I think
"Config" conveys "app maker config" given the name of the project, and
having "build" or "make" in the type name might be more confusing since
both could be read as a verb.
4. I tried replacing the derived properties with getters as suggested in
[1], but to be compatible with the Handlebars template compilation [2]
I think we would need to add a custom toJSON method to the class for
the derived properties, which was making the class increasingly harder
to read and understand vs. just outputting a config object including
the derived properties as I did. IMO this is an acceptable trade-off
given the config object isn’t really a user-facing thing, but let me
know if there’s a better way here.
[1]: #8 (comment)
[2]: https://github.com/Jigsaw-Code/outline-app-maker/blob/7fa63ea3539a105e66d8ee43791166ed21c5bef9/wrapper_app_project/scripts/build.mjs#L107-L1141 parent 7fa63ea commit ec8e4f1
File tree
10 files changed
+370
-272
lines changed- wrapper_app_project/scripts
10 files changed
+370
-272
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
4 | 5 | | |
5 | 6 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
This file was deleted.
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
9 | 10 | | |
| 11 | + | |
10 | 12 | | |
11 | 13 | | |
12 | 14 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
0 commit comments