From 7a3f26145f670e9f2deedeed0d5d36dcd25b29c0 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 12 Jul 2026 16:40:26 +0000 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Serve=20static=20assets=20?= =?UTF-8?q?via=20html=5Fstatic=5Fpath?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hand-rolled update_css_js/update_css_links handlers with a single builder-inited hook that appends sphinx_design/static/ to html_static_path, letting Sphinx copy, checksum and cache-bust the CSS/JS itself. Non-HTML builders no longer gain a spurious _sphinx_design_static directory in their output (#200, #235). Bump the Sphinx floor to >=7.2 (native CSS checksums since 7.1) and replace the docutils findall compat shim with direct node.findall calls. --- .pre-commit-config.yaml | 2 +- AGENTS.md | 4 +- CHANGELOG.md | 10 +++ package.json | 2 +- pyproject.toml | 2 +- sphinx_design/_compat.py | 10 --- sphinx_design/cards.py | 5 +- sphinx_design/dropdown.py | 5 +- sphinx_design/extension.py | 60 ++++------------ .../sd_tabs.js => static/design-tabs.js} | 0 .../sphinx-design.min.css} | 0 sphinx_design/tabs.py | 3 +- tests/conftest.py | 13 +--- tests/test_assets.py | 71 +++++++++++++++++++ tests/test_misc.py | 17 +++-- 15 files changed, 113 insertions(+), 91 deletions(-) rename sphinx_design/{compiled/sd_tabs.js => static/design-tabs.js} (100%) rename sphinx_design/{compiled/style.min.css => static/sphinx-design.min.css} (100%) create mode 100644 tests/test_assets.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f01b9ea0..0dc6560d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,7 +45,7 @@ repos: entry: npm run css require_serial: true pass_filenames: false - # args: [--style=compressed, --no-source-map, style/index.scss, sphinx_design/compiled/style.min.css] + # args: [--style=compressed, --no-source-map, style/index.scss, sphinx_design/static/sphinx-design.min.css] - id: tsc name: tsc (jsdoc) diff --git a/AGENTS.md b/AGENTS.md index eec9d11a..c2203917 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -294,7 +294,7 @@ The extension uses SASS for styling: 1. SASS source files are in `style/` 2. Compiled using `npm run css` (requires Node.js) -3. Output goes to `sphinx_design/compiled/style.min.css` +3. Output goes to `sphinx_design/static/sphinx-design.min.css` 4. CSS is automatically copied to build output during Sphinx builds ## Key Files @@ -339,7 +339,7 @@ The extension uses SASS for styling: 1. Edit SASS files in `style/` 2. Run `npm run css` to compile (or `pre-commit run --all css`) -3. Compiled output goes to `sphinx_design/compiled/style.min.css` +3. Compiled output goes to `sphinx_design/static/sphinx-design.min.css` 4. Test with different themes to ensure compatibility ## Reference Documentation diff --git a/CHANGELOG.md b/CHANGELOG.md index 9891a798..ea67ad97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ## Unreleased +- ♻️ IMPROVE: Static assets (CSS/JS) are now served via Sphinx's standard + `html_static_path` mechanism, rather than being written directly into the + build output; non-HTML builds no longer gain a spurious + `_sphinx_design_static` directory ({pr}`276`, {issue}`200`, {issue}`235`). + A stale `_sphinx_design_static` directory left in an existing HTML build + directory by previous versions is unused and can safely be deleted. +- ⬆️ UPGRADE: Sphinx `>=7.2` is now required ({pr}`276`) +- 🗑️ REMOVE: The private `sphinx_design._compat.findall` helper has been + removed (docutils `Element.findall` is guaranteed by the Sphinx floor); + any code importing it should call `node.findall(...)` directly ({pr}`276`) - 🐛 FIX: buttons are no longer destroyed by gettext translation: translated `button-link`/`button-ref` keep their styling and links (gettext now targets only the button text), thanks to {user}`sneakers-the-rat` diff --git a/package.json b/package.json index 9f27dacf..4fed328e 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "version": "0.0.1", "description": "Scripts for compiling the sphinx-design assets", "scripts": { - "css": "sass --style=compressed --no-source-map style/index.scss sphinx_design/compiled/style.min.css" + "css": "sass --style=compressed --no-source-map style/index.scss sphinx_design/static/sphinx-design.min.css" }, "dependencies": { "sass": "^1.35.2" diff --git a/pyproject.toml b/pyproject.toml index c08b2d48..344beb6e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,7 +27,7 @@ classifiers = [ ] keywords = ["sphinx", "extension", "material design", "web components"] requires-python = ">=3.11" -dependencies = ["sphinx>=7,<10"] +dependencies = ["sphinx>=7.2,<10"] [project.urls] Homepage = "https://github.com/executablebooks/sphinx-design" diff --git a/sphinx_design/_compat.py b/sphinx_design/_compat.py index e909d1ed..7d263e19 100644 --- a/sphinx_design/_compat.py +++ b/sphinx_design/_compat.py @@ -1,17 +1,7 @@ """Helpers for cross compatibility across dependency versions.""" -from collections.abc import Callable, Iterable from importlib import resources -from docutils.nodes import Element - - -def findall(node: Element) -> Callable[..., Iterable[Element]]: - """Iterate through""" - # findall replaces traverse in docutils v0.18 - # note a difference is that findall is an iterator - return getattr(node, "findall", node.traverse) - def read_text(module: resources.Package, filename: str) -> str: return resources.files(module).joinpath(filename).read_text() diff --git a/sphinx_design/cards.py b/sphinx_design/cards.py index e9abc7f2..97140b15 100644 --- a/sphinx_design/cards.py +++ b/sphinx_design/cards.py @@ -9,7 +9,6 @@ from sphinx.util.docutils import SphinxDirective from sphinx.util.logging import getLogger -from ._compat import findall from .shared import ( WARNING_TYPE, PassthroughTextElement, @@ -247,9 +246,9 @@ def _create_component( @staticmethod def add_card_child_classes(node): """Add classes to specific child nodes.""" - for para in findall(node)(nodes.paragraph): + for para in node.findall(nodes.paragraph): para["classes"] = [*para.get("classes", []), "sd-card-text"] - # for title in findall(node)(nodes.title): + # for title in node.findall(nodes.title): # title["classes"] = ([] if "classes" not in title else title["classes"]) + [ # "sd-card-title" # ] diff --git a/sphinx_design/dropdown.py b/sphinx_design/dropdown.py index 7a2ad8e0..1de2736f 100644 --- a/sphinx_design/dropdown.py +++ b/sphinx_design/dropdown.py @@ -16,7 +16,6 @@ margin_option, ) -from ._compat import findall from .icons import get_octicon, list_octicons @@ -153,7 +152,7 @@ class DropdownHtmlTransform(SphinxPostTransform): def run(self, **kwargs: Any) -> None: """Run the transform""" document: nodes.document = self.document - for node in findall(document)(lambda node: is_component(node, "dropdown")): + for node in document.findall(lambda node: is_component(node, "dropdown")): # TODO option to not have card css (but requires more formatting) use_card = True @@ -230,7 +229,7 @@ def run(self, **kwargs: Any) -> None: children=body_children, ) if use_card: - for para in findall(body_node)(nodes.paragraph): + for para in body_node.findall(nodes.paragraph): para["classes"] = ([] if "classes" in para else para["classes"]) + [ "sd-card-text" ] diff --git a/sphinx_design/extension.py b/sphinx_design/extension.py index a20915cb..d32c4814 100644 --- a/sphinx_design/extension.py +++ b/sphinx_design/extension.py @@ -1,17 +1,12 @@ from contextlib import contextmanager from functools import partial -import hashlib from pathlib import Path from docutils import nodes from docutils.parsers.rst import directives -from sphinx import version_info as sphinx_version from sphinx.application import Sphinx -from sphinx.environment import BuildEnvironment from sphinx.transforms import SphinxTransform -from . import compiled as static_module -from ._compat import findall, read_text from .article_info import setup_article_info from .badges_buttons import setup_badges_and_buttons from .cards import setup_cards @@ -27,12 +22,13 @@ ) from .tabs import setup_tabs +STATIC_DIR = Path(__file__).parent / "static" + def setup_extension(app: Sphinx) -> None: """Set up the sphinx extension.""" setup_sd_config(app) - app.connect("builder-inited", update_css_js) - app.connect("env-updated", update_css_links) + app.connect("builder-inited", add_static_assets) # we override container html visitors, to stop the default behaviour # of adding the `container` class to all nodes.container app.add_node( @@ -77,45 +73,13 @@ def _add_directive(name, directive, **kwargs): app.add_directive = add_directive # type: ignore[method-assign] -def update_css_js(app: Sphinx): - """Copy the CSS to the build directory.""" - # reset changed identifier - app.env.sphinx_design_css_changed = False # type: ignore[attr-defined] - # setup up new static path in output dir - static_path = (Path(app.outdir) / "_sphinx_design_static").absolute() - static_existed = static_path.exists() - static_path.mkdir(exist_ok=True) - app.config.html_static_path.append(str(static_path)) - # Copy JS to the build directory. - js_path = static_path / "design-tabs.js" - app.add_js_file(js_path.name) - if not js_path.exists(): - content = read_text(static_module, "sd_tabs.js") - js_path.write_text(content) - # Read the css content and hash it - content = read_text(static_module, "style.min.css") - # Write the css file - if sphinx_version < (7, 1): - hash = hashlib.md5(content.encode("utf8"), usedforsecurity=False).hexdigest() - css_path = static_path / f"sphinx-design.{hash}.min.css" - else: - # since sphinx 7.1 a checksum is added to the css file URL, so there is no need to do it here - # https://github.com/sphinx-doc/sphinx/pull/11415 - css_path = static_path / "sphinx-design.min.css" - app.add_css_file(css_path.name) - if css_path.exists(): +def add_static_assets(app: Sphinx) -> None: + """Register the extension's static assets (HTML-format builders only).""" + if app.builder.format != "html": return - if static_existed: - app.env.sphinx_design_css_changed = True # type: ignore[attr-defined] - for path in static_path.glob("*.css"): - path.unlink() - css_path.write_text(content, encoding="utf8") - - -def update_css_links(app: Sphinx, env: BuildEnvironment): - """If CSS has changed, all files must be re-written, to include the correct stylesheets.""" - if env.sphinx_design_css_changed: # type: ignore[attr-defined] - return list(env.all_docs.keys()) + app.config.html_static_path.append(str(STATIC_DIR)) + app.add_css_file("sphinx-design.min.css") + app.add_js_file("design-tabs.js") def visit_container(self, node: nodes.Node): @@ -175,15 +139,15 @@ class AddFirstTitleCss(SphinxTransform): def apply(self): hide = False - for docinfo in findall(self.document)(nodes.docinfo): - for name in findall(docinfo)(nodes.field_name): + for docinfo in self.document.findall(nodes.docinfo): + for name in docinfo.findall(nodes.field_name): if name.astext() == "sd_hide_title": hide = True break break if not hide: return - for section in findall(self.document)(nodes.section): + for section in self.document.findall(nodes.section): if isinstance(section.children[0], nodes.title): if "classes" in section.children[0]: section.children[0]["classes"].append("sd-d-none") diff --git a/sphinx_design/compiled/sd_tabs.js b/sphinx_design/static/design-tabs.js similarity index 100% rename from sphinx_design/compiled/sd_tabs.js rename to sphinx_design/static/design-tabs.js diff --git a/sphinx_design/compiled/style.min.css b/sphinx_design/static/sphinx-design.min.css similarity index 100% rename from sphinx_design/compiled/style.min.css rename to sphinx_design/static/sphinx-design.min.css diff --git a/sphinx_design/tabs.py b/sphinx_design/tabs.py index d1e92619..1b69530e 100644 --- a/sphinx_design/tabs.py +++ b/sphinx_design/tabs.py @@ -6,7 +6,6 @@ from sphinx.transforms.post_transforms import SphinxPostTransform from sphinx.util.logging import getLogger -from ._compat import findall from .shared import ( WARNING_TYPE, SdDirective, @@ -246,7 +245,7 @@ def run(self, **kwargs: Any) -> None: tab_item_id_num = 0 for tab_set_id_num, tab_set in enumerate( - findall(self.document)(lambda node: is_component(node, "tab-set")) + self.document.findall(lambda node: is_component(node, "tab-set")) ): tab_set_identity = tab_set_id_base + str(tab_set_id_num) children = [] diff --git a/tests/conftest.py b/tests/conftest.py index 28324c27..ad1a28e6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,19 +6,10 @@ from docutils import __version_info__ as docutils_version_info from docutils import nodes import pytest -from sphinx import version_info from sphinx.testing.util import SphinxTestApp -from sphinx_design._compat import findall - pytest_plugins = "sphinx.testing.fixtures" -if version_info >= (7, 2): - # see https://github.com/sphinx-doc/sphinx/pull/11526 - from pathlib import Path as sphinx_path # noqa: N813 -else: - from sphinx.testing.path import path as sphinx_path # type: ignore[no-redef] - class SphinxBuilder: def __init__(self, app: SphinxTestApp, src_path: Path): @@ -54,7 +45,7 @@ def get_doctree( if post_transforms: self.app.env.apply_post_transforms(doctree, docname) # make source path consistent for test comparisons - for node in findall(doctree)(include_self=True): + for node in doctree.findall(include_self=True): if not (hasattr(node, "get") and node.get("source")): continue node["source"] = Path(node["source"]).relative_to(self.src_path).as_posix() @@ -86,7 +77,7 @@ def _create_project( ) src_path.joinpath("conf.py").write_text(content, encoding="utf8") app = make_app( - srcdir=sphinx_path(os.path.abspath(str(src_path))), # noqa: PTH100 + srcdir=Path(os.path.abspath(str(src_path))), # noqa: PTH100 buildername=buildername, ) return SphinxBuilder(app, src_path) diff --git a/tests/test_assets.py b/tests/test_assets.py new file mode 100644 index 00000000..8679cf74 --- /dev/null +++ b/tests/test_assets.py @@ -0,0 +1,71 @@ +"""Tests for how the extension registers and emits its static assets (CSS/JS). + +These tests are intentionally written with plain reStructuredText and only the +``sphinx_design`` extension enabled, so that they also run in the +``py311-no-myst`` environment. +""" + +from pathlib import Path + +# Keep the config myst-free so these tests run without myst-parser installed. +SD_CONF = {"extensions": ["sphinx_design"]} + +INDEX_RST = """ +Test Document +============= + +Some content. +""" + + +def _write_index(src_path: Path) -> None: + src_path.joinpath("index.rst").write_text(INDEX_RST, encoding="utf8") + + +def test_latex_no_static_dir(sphinx_builder): + """A latex build must not gain a spurious ``_sphinx_design_static`` directory. + + Regression test for #200 (non-HTML builders getting the static dir) and + #235 (``mkdir`` crashing when the outdir does not yet exist). + """ + builder = sphinx_builder(buildername="latex", conf_kwargs=SD_CONF) + _write_index(builder.src_path) + builder.build() + out_path = builder.out_path + # the old hand-rolled copying wrote this directory for *every* builder + assert list(out_path.rglob("_sphinx_design_static")) == [] + # and the CSS should never leak into a non-HTML build + assert list(out_path.rglob("sphinx-design.min.css")) == [] + + +def test_html_static_assets(sphinx_builder): + """An HTML build copies the assets into ``_static`` and links them with a checksum.""" + builder = sphinx_builder(buildername="html", conf_kwargs=SD_CONF) + _write_index(builder.src_path) + builder.build() + out_path = builder.out_path + assert out_path.joinpath("_static", "sphinx-design.min.css").exists() + assert out_path.joinpath("_static", "design-tabs.js").exists() + index_html = out_path.joinpath("index.html").read_text(encoding="utf8") + # Sphinx >=7.1 appends a native ``?v=`` cache-busting suffix + assert "sphinx-design.min.css?v=" in index_html + assert "design-tabs.js?v=" in index_html + + +def test_epub_static_assets(sphinx_builder): + """An epub build still receives the assets (preserving pre-existing behaviour). + + ``Epub3Builder.format == "html"``, so it keeps getting the assets exactly as + before. A minimal epub project emits unrelated EPUB3 metadata warnings (empty + ``epub_copyright`` / ``version``), so we do not assert a warning-free build; + we only check that output was produced and the assets are present. + """ + builder = sphinx_builder(buildername="epub", conf_kwargs=SD_CONF) + _write_index(builder.src_path) + builder.build(assert_pass=False) + out_path = builder.out_path + # the build produced epub output + assert list(out_path.rglob("*.epub")) + # and the assets were copied into the epub tree + assert list(out_path.rglob("sphinx-design.min.css")) + assert list(out_path.rglob("design-tabs.js")) diff --git a/tests/test_misc.py b/tests/test_misc.py index c9f351b0..5e831693 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -6,7 +6,6 @@ from docutils import nodes import pytest -from sphinx_design._compat import findall from sphinx_design.config import SdConfig, get_sd_config from sphinx_design.icons import get_material_icon_data, get_octicon_data from sphinx_design.shared import is_component @@ -151,7 +150,7 @@ def test_grid_with_comment(fmt, sphinx_builder): ) builder.build() # asserts no warnings doctree = builder.get_doctree("index") - grid_items = list(findall(doctree)(lambda node: is_component(node, "grid-item"))) + grid_items = list(doctree.findall(lambda node: is_component(node, "grid-item"))) assert len(grid_items) == 2 @@ -209,7 +208,7 @@ def test_card_carousel_with_comment(fmt, sphinx_builder): ) builder.build() # asserts no warnings doctree = builder.get_doctree("index") - cards = list(findall(doctree)(lambda node: is_component(node, "card"))) + cards = list(doctree.findall(lambda node: is_component(node, "card"))) assert len(cards) == 2 @@ -268,7 +267,7 @@ def test_tab_set_with_comment(fmt, sphinx_builder): builder.build() # asserts no warnings # both tabs should still render in the HTML output doctree = builder.get_doctree("index", post_transforms=True) - inputs = list(findall(doctree)(sd_tab_input)) + inputs = list(doctree.findall(sd_tab_input)) assert len(inputs) == 2 assert inputs[0]["checked"] is True @@ -326,16 +325,16 @@ def test_tab_set_with_target(fmt, sphinx_builder): doctree = builder.get_doctree("index", post_transforms=True) # the target survives both the directive and HTML transform rebuilds, # placed at the front of the tab-set - targets = list(findall(doctree)(nodes.target)) + targets = list(doctree.findall(nodes.target)) assert len(targets) == 1 assert is_component(targets[0].parent, "tab-set") assert targets[0].parent.children[0] is targets[0] # the tab still renders and is selected by default - inputs = list(findall(doctree)(sd_tab_input)) + inputs = list(doctree.findall(sd_tab_input)) assert len(inputs) == 1 assert inputs[0]["checked"] is True # the reference resolves to the target - references = list(findall(doctree)(nodes.reference)) + references = list(doctree.findall(nodes.reference)) assert len(references) == 1 # crucially, some rendered node must still CARRY the id # (docutils PropagateTargets moves it onto the tab-item container, @@ -343,7 +342,7 @@ def test_tab_set_with_target(fmt, sphinx_builder): refid = references[0]["refid"] carriers = [ node - for node in findall(doctree)() + for node in doctree.findall() if isinstance(node, nodes.Element) and not isinstance(node, nodes.target) and refid in node.get("ids", []) @@ -401,7 +400,7 @@ def test_tab_set_code_with_target(sphinx_builder): ) builder.build() # asserts no warnings doctree = builder.get_doctree("index", post_transforms=True) - references = [ref for ref in findall(doctree)(nodes.reference) if ref.get("refid")] + references = [ref for ref in doctree.findall(nodes.reference) if ref.get("refid")] assert len(references) == 1 refid = references[0]["refid"] html = (builder.out_path / "index.html").read_text(encoding="utf8")