Skip to content

♻️ Refactor percy/config utils#406

Merged
wwilsman merged 8 commits intomasterfrom
ww/config-util-refactor
Jul 15, 2021
Merged

♻️ Refactor percy/config utils#406
wwilsman merged 8 commits intomasterfrom
ww/config-util-refactor

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

@wwilsman wwilsman commented Jul 9, 2021

Purpose

While adding better validations for config options and per-snapshot validations, data must be validated against schemas other than the global config schema. To facilitate this, the validate util was refactored slightly. The merge util is also used within the core snapshot options config function to merge and validate provided options with config options and defaults while also handling deprecated options. Due to the merge util's limitation of only merging two objects at a time while mutating the first object, it had to be used several times in a row in order to merge all of the snapshot options together into a new object. Furthermore, it's options argument was limited in it's abilities and the merge function itself was centered around config normalization, so wasn't generic enough to use outside of said normalization function.

Approach

The validate utils were refactored so default schema $ids now start with a forward slash (/) to help distinguish them from local schema references. The function will also now only return an array of errors on failure and undefined on success. The associated addSchema util was updated to accept schemas without automatically adding them to the global Percy config schema (via an explicit $id). This allows the validate util to be used with other registered schemas and not just the global Percy config schema.

The merge util was refactored to accept an array of objects instead of only two objects. It also always returns a deep clone rather than mutating the first argument. The limited options argument was replaced with a map callback argument. The callback will be called for each property within each object, recursively, with the path, previous value, and next value of the property. A path-value pair can be returned to override the default merge behavior. Some options previously handled by the old merge function were replaced by the new map callback in the appropriate places.

Other features were also added while refactoring that were found to be useful for better validations:

  • Schemas using minimum and maximum keywords will be clamped rather than scrubbed.
  • Schemas can use a custom instanceof keyword to allow functions or regular expressions.
  • Schemas can also use a custom disallowed keyword, based on the built-in required keyword (but negated).
  • Nested objects that have missing required properties are now scrubbed.
  • Empty properties such as array items that are missing or deleted are now scrubbed.
    e.g. ['foo', <5 empty items>, 'xyzzy'] => ['foo', 'xyzzy']
  • Validation error paths were updated to show as proper array notation.
    before: snapshots.widths.0: must be <= 2000
    after: snapshots.widths[0]: must be <= 2000

wwilsman added 8 commits July 8, 2021 13:56
The underlying merge util was refactored to accept an array of objects instead of only two
objects. The options argument was replaced with a map callback argument. The function will be called
for each property within each object recursively with the path, previous value, and next value of
the property. A path-value pair can be returned to override the default merge behavior.

Some options previously handled by the old merge function were replaced by the new map callback in
the appropriate places.

The validate util was also refactored, though to a much less degree. Default schema $id's now start
with a forward slash to help disguish them from local schema references. The function will also now
only return an array of errors on failure and undefined on success. The associated addSchema util
was also updated to accept schema's without automatically adding them to the global Percy config
schema. This allows the validate util to be used with other registered schemas and not just the
global Percy config schema.
Heavily based on the required keyword, destructured from internal helpers, and negated since we want
to disallow properties rather than require them.
@wwilsman wwilsman added the ✨ enhancement New feature or request label Jul 9, 2021
@wwilsman wwilsman requested a review from Robdel12 July 9, 2021 00:21
@wwilsman wwilsman enabled auto-merge (squash) July 9, 2021 00:24
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.

🏁

@wwilsman wwilsman merged commit 980fb7b into master Jul 15, 2021
@wwilsman wwilsman deleted the ww/config-util-refactor branch July 15, 2021 15:39
samarsault pushed a commit that referenced this pull request Mar 3, 2023
Bumps [cypress](https://github.com/cypress-io/cypress) from 8.3.0 to 8.3.1.
- [Release notes](https://github.com/cypress-io/cypress/releases)
- [Changelog](https://github.com/cypress-io/cypress/blob/develop/.releaserc.base.js)
- [Commits](cypress-io/cypress@v8.3.0...v8.3.1)

---
updated-dependencies:
- dependency-name: cypress
  dependency-type: direct:development
  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