Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove references to a version number for theme.json #1907

Conversation

ryanwelcher
Copy link
Contributor

@ryanwelcher ryanwelcher commented Oct 28, 2021

This PR removes the V1 references in the WordPress theme.json schema.

In my opinion, it's better to have the canonical URL be https://json.schemastore.org/theme.json and manage any changes via deprecation notices in the schema.

This mirrors how the changes for https://json.schemastore.org/block.json (#1898) have been done and will allow automated tools to reference the schema at the same location regardless of version.

@ryanwelcher
Copy link
Contributor Author

cc @fabiankaegy @ajlende @mkaz

@mkaz
Copy link

mkaz commented Oct 28, 2021

My concern would be breaking existing references, is there a way to redirect the old to the new?

@GerryFerdinandus
Copy link
Contributor

Redirect old schema to new

Create the 'old' theme-v1.json that redirect to the 'new' theme.json

{
  "title": "JSON schema for WordPress block theme global settings and styles",
  "$schema": "http://json-schema.org/draft-04/schema#",
  "$ref": "https://json.schemastore.org/theme.json"
}

Add an extra line at the top of theme.json file.
"id": "https://json.schemastore.org/theme.json",

The catalog file must contain both the files theme.json and theme-v1.json

    {
      "name": "theme.json",
      "description": "WordPress block theme global settings and styles configuration file version 1",
      "fileMatch": [
        "theme.json"
      ],
      "url": "https://json.schemastore.org/theme.json",
      "versions": {
        "1.0": "https://json.schemastore.org/theme-v1.json"
      }
    },

Tell the AJV validator where the the external file is.
Add this extra line in schema-validation.json "options": [] list

    {
      "theme-v1.json": {
        "externalSchema": [
          "theme.json"
        ]
      }
    },

@ajlende
Copy link
Contributor

ajlende commented Oct 28, 2021

My thought behind adding the v1 was so that we could keep the schema clean when properties are removed. Unlike block.json, the documentation for theme.json suggested that the version property would be incremented when there were breaking changes, and the plugin would be able to migrate to the new version.

WordPress 5.8 will ignore the contents of any theme.json whose version is not equals to the current. Should the Gutenberg plugin need it, it’ll update the version and will add the corresponding migration mechanisms from older versions.

https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-json/#version

When we increment to version 2, v1 can still exist, and v2 won't have to be filled with deprecated properties.

@ryanwelcher
Copy link
Contributor Author

My thought behind adding the v1 was so that we could keep the schema clean when properties are removed. Unlike block.json, the documentation for theme.json suggested that the version property would be incremented when there were breaking changes, and the plugin would be able to migrate to the new version.

When we increment to version 2, v1 can still exist, and v2 won't have to be filled with deprecated properties.

These are all great points. I do believe that having a single URL with deprecated properties turns this into more of a living document and really helps with developer education and the overall developer experience.

For example, if I was unaware that a new version had been released, I won't know to update the $schema entry in my theme.json and I could be using deprecated properties that will be flagged during the development process and will also not be aware of new properties being added. This approach puts the owners on the developer to research and update as needed. It will also cause some frustration and loss of productivity trying to research how to fix the problem.

Conversely, if we have a single URL with all of the deprecated properties, a developer will go to add the property they want and would see a message to the effect of Deprecated: PropertyName has been moved/replaced .... in their IDE. This approach requires no additional effort for the developer, helps with the education and adoption of changes to the API, and "just works".

@ajlende
Copy link
Contributor

ajlende commented Oct 29, 2021

The scenario that I imagined was more like this:

The theme.json version is updated in a new WordPress version, and I am unaware of this change. Because of the automatic migration, my theme still works just fine. Maybe there is a warning printed somewhere, and maybe I still don't see that.

Later, I'm adding a new feature to my theme because I read a blog post about it or something. I add the new property to the theme.json, and the new property gets undercurls to indicate that something is wrong.

If the future upgrade from v1 to v2 works the same as v0 (experimental-theme.json) to v1 (theme.json), any new properties will be ignored unless the version property is updated.

It’s important to include the “version” attribute; otherwise, the data will be parsed as “version 0”, which differs greatly from the expected behavior outlined here.

If I ignore the undercurl and run the theme, the new property won't work because I haven't updated the version number. The undercurl is a flag that says "there is something wrong with this part", and I will end up searching the documentation (or just the web in general) for why the property I added isn't working. Hopefully, the documentation (or someone's blog post) points to it being a property only available in version 2.

I find the documentation for the update, so I update the version property in theme.json to 2. At this point, the version property has an undercurl because it's a required property and in v1 the value must be 1.

That leads me to update the $schema version to v2 and a bunch more undercurls show up. Everywhere that has an undercurl needs to be updated for v2 or the property will be ignored.

I follow the undercurls with the help of the documentation (maybe a v1 to v2 migration page/section like we have for add_theme_support) and end up with a correct theme.json v2.


When updating from v0 to v1, I had a point where I couldn't figure out why my theme wasn't working and eventually I figured out it was because I forgot to add the version property. This differs greatly from how block.json is versioned. Having the undercurls would have helped me pinpoint the problem faster, I think.

Searching the documentation isn't great and may cause some frustration like you said, but I think it's better to have obvious feedback that something is wrong rather than relying on hovering over every property to read the tooltip since there are so many properties and more will continue to be added in the future.


I wonder if @oandregal can share plans for how new versions will be supported in theme.json because if my idea of how it will work is wrong, then the scenario will play out differently and we may want to handle this differently.


Also worth noting that @mkaz is working on adding the schemas to the Wordpress/gutenberg repo, so we would get versioning in the form of tags if/when that happens.

@oandregal
Copy link

Yeah, so what Alex shared is how it works. Essentially:

  • v1 is tied to WordPress 5.8.
  • v2 will be tied to WordPress 5.9.
  • Subsequent WordPress versions (major, minor, or bugfixes) can have new versions as necessary
  • The Gutenberg plugin acts as a "evergreen" place where the theme.json is always the latest.

When changes are made that aren't back compatible (removals, renames, reorganization, etc.) we will add a migration so older versions still work. This means that there will be different valid theme.json schemas at the same time. It also means that themes that use an older theme.json version will still work in WordPress versions that use a newer one.

Hope this helps :)

@ryanwelcher
Copy link
Contributor Author

Thanks for adding the detailed explanation @ajlende and @oandregal! It's given me a much better understanding of the whole process.

@ajlende
Copy link
Contributor

ajlende commented Nov 11, 2021

I opened a PR to reference the WordPress repo source (#1930). It also removes the version reference in the catalog, so if that one gets merged, we can close this one.

@ryanwelcher
Copy link
Contributor Author

Closing now that #1930 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants