Merged
Conversation
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.
Robdel12
approved these changes
Jul 29, 2021
Contributor
Robdel12
left a comment
There was a problem hiding this comment.
🏁 I love how much stronger all the SDKs are getting from the Storybook SDK improvements
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this?
With #447, the new
static.rewritesconfig 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 normalizingdiscovery.request-headersoptions, and subsequently laterdiscovery.cookiesoptions.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.normalizemethod now accepts another option which can define a schema to normalize against. In order to get a property's associated schema, thegetSchemavalidation 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
$refin 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 likeoneOf, 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
getSchemaand minornormalizechanges, themigratehelper 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
loadmethod 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