Skip to content

✨ Normalize config with schemas#450

Merged
wwilsman merged 15 commits intomasterfrom
ww/normalize-config-with-schema
Jul 29, 2021
Merged

✨ Normalize config with schemas#450
wwilsman merged 15 commits intomasterfrom
ww/normalize-config-with-schema

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

@wwilsman wwilsman commented Jul 29, 2021

What is this?

With #447, the new static.rewrites config file option was added to a config module constant that determines which object properties are normalized and which aren't. Before that PR, the constant was initially created to prevent normalizing discovery.request-headers options, and subsequently later discovery.cookies options.

This was all easy to do as the changes came up since the aforementioned options were defined by other packages within this repo. However, outside of this repo where config schemas can still be defined, there is no such mechanism for disabling normalization of a property. In order to do so, a dedicated PR must be opened on this repo to add the desired properties to the constant.

Instead, with this PR, normalization can be disabled for a property by its own schema. The PercyConfig.normalize method now accepts another option which can define a schema to normalize against. In order to get a property's associated schema, the getSchema validation helper was updated to accept a path. Upon providing a path, the function will now return an array of schema's indexed by each part of the path, with the zero indexed item being the root schema.

If a path is provided, subsequent nested schemas will be accumulated with the original root schema by recursively fetching the next path's schema. To facilitate schema references, the base functionality of this method will now resolve any $ref in a provided schema by once again recursively fetching the desired schema and plucking out the referenced meta path. For schemas that actually contain more than a single possible schema, such as tuples or meta schemas like oneOf, the best matching schema is determined by how deep the paths match; with a bias towards loosely matched schemas that can contain properties not defined by said schema.

After the extensive getSchema and minor normalize changes, the migrate helper was also updated to accept a schema to migrate against (defaulting to the default config schema). Since normalization can be dependent on a schema, and because migrations normalize their data, the migrate helpers were updated to allow defining migrations for specific schemas. While this won't be utilized immediately for migrating other options, it can definitely come in handy for future deprecations or breaking changes.

In addition to the changes described above, the test that was added also uncovered a merge bug that was fixed by re-contextualizing the merge if the merge path is mapped to another path. Before, when mapping parent keys of arrays, the context was lost which could trick the following logic into thinking it was creating a new array rather than concatenating arrays. An extra normalization usage in the load method was also removed because empty values as a result of pruning invalid options are already removed by validation changes in #406.

Because of the changes in this PR, and in addition to those validation changes, other small codepaths were (re)moved. There was an old codepath for duplicate "using config" logs that was handled in the core constructor. With the new changes, we can move this to within the load method itself. If a file is not loaded, the log will not happen; but validation will still be performed in addition to merging with defaults as the old code path did. This actually improves core config options by always validating, even when not loaded by a file. Another code path, which normalized snapshot options, was moved into the snapshot options config helper introduced in #416.


Me adding new config options to the Storybook SDK: https://www.youtube.com/watch?v=AbSehcT19u0

wwilsman added 15 commits July 28, 2021 18:54
Empty values as a result of pruning invalid options are now already removed
When mapping parent keys of arrays the context is lost, which can trick the following logic into
thinking it is creating a new array rather than concatenating arrays.
If a path is provided, subsequent nested schemas will be accumulated with the original root schema
by recursively fetching the next path's schema. To facilitate schema references, the base
functionality of this method will now resolve any `$ref` in a provided schema by once again
recursively fetching the desired schema and plucking out the referenced meta path. For schemas that
actually contain more than a single possible schema, such as tuples or meta schemas like `oneOf`,
the best matching schema is determined by how deep the paths match; with a bias towards loosely
matched schemas that can contain properties not defined by said schema.
This eliminates the need for a constant to determine which objects should not have their properties
normalized. While this constant is easy to update for internal packages, it can cause issues for
external packages that require certain properties not be normalized.
Now that a property's normalization can depend on its schema, the migrate function can now also accept
a schema name so it can properly normalize any incoming data.

Since normalization can be dependant on a schema, and because migrations normalize their data, the
migrate helpers were updated to allow defining migrations for specific schemas. While this won't be
utilized immediately for migrating other options, it can definitely come in handy for future
deprecations or breaking changes.
The load method has the capability to not load files, but the "using config" log would log twice if
the load method was called twice. The solution for silencing the duplicate log was to not call load
a second time and instead just load defaults. This prompted a test for this new code path for
coverage at the time.

However, now that validation and normalization have changed, we can adjust this within the load
method itself. If a file is not loaded, the log will not happen; but validation will still be
performed within the load method in addition to merging with default as the old additional code path
did. This actually improves additional config file options by always validating, even when not
loaded by a config file.
@wwilsman wwilsman added the ✨ enhancement New feature or request label Jul 29, 2021
@wwilsman wwilsman requested a review from Robdel12 July 29, 2021 03:10
@wwilsman wwilsman enabled auto-merge (squash) July 29, 2021 15:29
Copy link
Copy Markdown
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁 I love how much stronger all the SDKs are getting from the Storybook SDK improvements

@wwilsman wwilsman merged commit 146d4bc into master Jul 29, 2021
@wwilsman wwilsman deleted the ww/normalize-config-with-schema branch July 29, 2021 15:59
samarsault pushed a commit that referenced this pull request Mar 3, 2023
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 2.5.0 to 2.5.1.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.5.0...v2.5.1)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants