Fixed extra:analytics schema for custom providers

[sisp]

I've fixed the JSON schema for specifying custom providers which don't use the property property like Google Analytics and/or have additional properties.

This is a set of test cases that show the current validation results in VS Code:

site_name: Docs with analytics

theme:
  name: material

extra:
  # expected: ✅ / actual ✅
  analytics:
    provider: google
    property: G-XXXXXXXXXX

  # expected: ✅ / actual ✅
  analytics:
    provider: google
    property: G-XXXXXXXXXX
    feedback:
      title: Was this page helpful?

  # expected: ❌ / actual: ❌
  analytics: # 🚩 Missing property "property". yaml-schema: Analytics provider
    provider: google
    property2: G-XXXXXXXXXX # 🚩 Property property2 is not allowed. yaml-schema: Analytics provider

  # expected: ❌ / actual: ❌
  analytics: # 🚩 Missing property "property". yaml-schema: Analytics provider
    provider: google
    property2: G-XXXXXXXXXX # 🚩 Property property2 is not allowed. yaml-schema: Analytics provider
    feedback:
      title: Was this page helpful?

  # expected: ✅ / actual: ❌
  analytics: # 🚩 Missing property "property". yaml-schema: Analytics provider
    provider: custom

  # expected: ✅ / actual: ❌
  analytics: # 🚩 Missing property "property". yaml-schema: Analytics provider
    provider: custom
    feedback:
      title: Was this page helpful?

  # expected: ✅ / actual: ✅
  analytics:
    provider: custom
    property: custom-property

  # expected: ✅ / actual: ✅
  analytics:
    provider: custom
    property: custom-property
    feedback:
      title: Was this page helpful?

  # expected: ✅ / actual: ❌
  analytics: # 🚩 Missing property "property". yaml-schema: Analytics provider
    provider: plausible
    domain: example.com # 🚩 Property domain is not allowed. yaml-schema: Analytics provider

  # expected: ✅ / actual: ❌
  analytics: # 🚩 Missing property "property". yaml-schema: Analytics provider
    provider: plausible
    domain: example.com # 🚩 Property domain is not allowed. yaml-schema: Analytics provider
    feedback:
      title: Was this page helpful?

With the changed schema in this PR, all test cases yield the expected results.

Before, the schema didn't couple provider: google with making property required, and as a side-effect, property was always defined using a schema specific to Google Analytics. Further, additionalProperties: false prevented any other custom properties that may be needed for some analytics solution.

Now, provider: google, property with the Google-specific schema, declaring both as required properties, and disallowing additional properties are all coupled. For any other provider: <string>, additional properties are allowed without defined schemas. The probably obvious approaches to express this with JSON Schema are to use

• oneOf, which doesn't work because the schema for provider: <string> is a superset of provider: google, so more than one schema matches, or
• anyOf as an attempt at fixing the problem with oneOf, but then the provider: <string> schema matches always and the additional constraints from the provider: google schema don't apply.

The solution I've implemented uses a combination of allOf and if-then (without else), such that any valid config of a defined provider (only Google Analytics at the moment) matches both the corresponding schema and the provider: <string> schema, whereas a config of a custom provider matches only the provider: <string> schema. Note that it's important to declare also the feedback property in each allOf schema of a specific provider because additionalProperties: false limits the set of valid properties only to those defined in the same schema and doesn't recognize the feedback property schema in the parent schema of allOf. But for clarity, I've also declared the feedback property in the provider: <string> schema, although it isn't necessary as the missing additionalProperties implies true, so feedback would be allowed anyway.

I hope the rationale of the solution, combined with the PR diff, is somewhat clear, although I'm not sure I was able to explain it so well.

WDYT, @squidfunk?

[squidfunk]

LGTM, only some minor changes. I think it is a good idea to make this more flexible!

[sisp]

Thanks for the feedback! I've updated the PR as you requested.

[sisp]

Just made the second commit message past tense since that's how you do it. 😉

[squidfunk]

Just made the second commit message past tense since that's how you do it. 😉

Thanks for sticking closely to our conventions! In fact, I'm using present tense on new projects, but I keep them past for consistency here, since that's how we roll since many years.