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

Theme.json: create a consistent syntax for relative paths #61804

Open
ramonjd opened this issue May 20, 2024 · 4 comments
Open

Theme.json: create a consistent syntax for relative paths #61804

ramonjd opened this issue May 20, 2024 · 4 comments
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Code Quality Issues or PRs that relate to code quality

Comments

@ramonjd
Copy link
Member

ramonjd commented May 20, 2024

What problem does this address?

A convention was introduced as part of the web fonts feature, with regards to relative paths to theme assets in theme.json.

The convention is that a prefix of file:./ is required to indicate that a path is relative to the theme root directory.

For example, the following path —

"src": "file:./assets/some/file.ext"

— assumes that /assets/ is top-level directory of the theme.

This rule also applies to style variations, so theme.json files in the /styles folder.

This convention was followed for background image URIs.

See the discussions in:

What is your proposed solution?

As @noisysocks points out:

Relative paths using . as a prefix is misleading because the paths are actually relative to the theme directory and not to where the theme.json file is. It also implies that .. should work which it doesn't. We should make the paths actually relative or remove support for them. This is an issue for fonts as well.

I would propose either:

  1. Communicate that all paths, regardless of the depth, must be relative to the theme root, and perhaps also support "file:" (without dotslash to avoid confusion) by default and have backwards compat handling for "file:./"
  2. Allow paths that are relative to current directory, and then in resolve them in the background with PHP trickery with realpath or something.

As @creativecoder notes, we should update the docs to reflect the current behaviour:

https://developer.wordpress.org/themes/global-settings-and-styles/settings/typography/#registering-web-fonts-font-faces

For example:

The src property is unique in that it allows you to reference a URL that is relative to the theme's root directory, regardless of where the theme.json resides. For example, to reference a font bundled with your theme, you would use the "file:./path/to/file.ext" format in both the theme's main theme.json or in theme variation JSON files in the /styles directory.

@ramonjd ramonjd added [Type] Code Quality Issues or PRs that relate to code quality [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. labels May 20, 2024
@ramonjd
Copy link
Member Author

ramonjd commented May 20, 2024

Also, should we consider only saving relative paths to the database? This applies to fonts as well:

@noisysocks
Copy link
Member

noisysocks commented May 27, 2024

I'm not sure what the best solution is given we've put ourselves into a corner re. backwards compatibility 😅

Actually supporting relative paths seems problematic since it implies that .. would work which it doesn't, and I'm not sure that we ever should support .. since we don't want to allow accessing files outside of the theme directory.

So I suppose best to either keep things as they are and document it, or change the prefix to be file: or file:/ instead of file:./.

@ramonjd
Copy link
Member Author

ramonjd commented May 27, 2024

I'm not sure what the best solution is given we've put ourselves into a corner re. backwards compatibility
Actually supporting relative paths seems problematic since it implies that .. would work which it doesn't, and I'm not sure that we ever should support .. since we don't want to allow accessing files outside of the theme directory.

Thanks for the feedback! I agree 💯

I think I'd therefore support point 1 in the description ("Communicate that all paths, regardless of the depth, must be relative to the theme root, and perhaps also support "file:" (without dotslash to avoid confusion) by default and have backwards compat handling for "file:./"")

@creativecoder
Copy link
Contributor

  1. Communicate that all paths, regardless of the depth, must be relative to the theme root, and perhaps also support "file:" (without dotslash to avoid confusion) by default and have backwards compat handling for "file:./"

I agree that option one seems like the best path here, at least for now. Any changes to support a new/different file syntax and maintain backcompat seem likely to add a lot of complexity, so I don't think there's enough benefit to warrant the added complexity at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

3 participants