-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add content hashes to asset URLs #6749
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Added content-hash cache busting to `rx.asset` URLs. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||
| """Helper functions for adding assets to the app.""" | ||||||||||||
|
|
||||||||||||
| import hashlib | ||||||||||||
| import inspect | ||||||||||||
| from pathlib import Path | ||||||||||||
| from typing import TYPE_CHECKING, overload | ||||||||||||
|
|
@@ -8,6 +9,9 @@ | |||||||||||
| from reflex_base.config import get_config | ||||||||||||
| from reflex_base.environment import EnvironmentVariables | ||||||||||||
|
|
||||||||||||
| _HASH_CHUNK_SIZE = 1024 * 1024 | ||||||||||||
| _MAX_HASH_ATTEMPTS = 3 | ||||||||||||
|
|
||||||||||||
| if TYPE_CHECKING: | ||||||||||||
| from typing_extensions import Buffer | ||||||||||||
|
|
||||||||||||
|
|
@@ -25,13 +29,21 @@ class AssetPathStr(str): | |||||||||||
| construction time. | ||||||||||||
| """ | ||||||||||||
|
|
||||||||||||
| __slots__ = ("importable_path",) | ||||||||||||
| __slots__ = ("_raw_path", "importable_path") | ||||||||||||
|
|
||||||||||||
| _raw_path: str | ||||||||||||
| importable_path: str | ||||||||||||
|
|
||||||||||||
| @overload | ||||||||||||
| def __new__(cls, object: object = "") -> "AssetPathStr": ... | ||||||||||||
| @overload | ||||||||||||
| def __new__( | ||||||||||||
| cls, | ||||||||||||
| object: object = "", | ||||||||||||
| *, | ||||||||||||
| importable_path: str | None = None, | ||||||||||||
| ) -> "AssetPathStr": ... | ||||||||||||
| @overload | ||||||||||||
| def __new__( | ||||||||||||
| cls, | ||||||||||||
| object: "Buffer", | ||||||||||||
|
|
@@ -44,6 +56,8 @@ def __new__( | |||||||||||
| object: object = "", | ||||||||||||
| encoding: str | None = None, | ||||||||||||
| errors: str | None = None, | ||||||||||||
| *, | ||||||||||||
| importable_path: str | None = None, | ||||||||||||
| ) -> "AssetPathStr": | ||||||||||||
| """Construct from an unprefixed, leading-slash asset path. | ||||||||||||
|
|
||||||||||||
|
|
@@ -56,6 +70,7 @@ def __new__( | |||||||||||
| object: The object to stringify (str, bytes, or any object). | ||||||||||||
| encoding: Encoding to decode ``object`` with when it is bytes-like. | ||||||||||||
| errors: Error handler for decoding. | ||||||||||||
| importable_path: Optional unversioned path to use for build-time imports. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| A new ``AssetPathStr`` instance. | ||||||||||||
|
|
@@ -72,7 +87,8 @@ def __new__( | |||||||||||
| instance = super().__new__( | ||||||||||||
| cls, get_config().prepend_frontend_path(relative_path) | ||||||||||||
| ) | ||||||||||||
| instance.importable_path = f"$/public{relative_path}" | ||||||||||||
| instance._raw_path = relative_path | ||||||||||||
| instance.importable_path = f"$/public{importable_path or relative_path}" | ||||||||||||
| return instance | ||||||||||||
|
|
||||||||||||
| def __getnewargs__(self) -> tuple[str]: | ||||||||||||
|
|
@@ -87,7 +103,57 @@ def __getnewargs__(self) -> tuple[str]: | |||||||||||
| Returns: | ||||||||||||
| A one-tuple containing the unprefixed asset path. | ||||||||||||
| """ | ||||||||||||
| return (self.importable_path[len("$/public") :],) | ||||||||||||
| return (self._raw_path,) | ||||||||||||
|
|
||||||||||||
| def __getnewargs_ex__(self) -> tuple[tuple[str], dict[str, str]]: | ||||||||||||
| """Return constructor args and kwargs for pickle/copy reconstruction. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| Constructor args and kwargs preserving the unversioned import path. | ||||||||||||
| """ | ||||||||||||
| return ( | ||||||||||||
| (self._raw_path,), | ||||||||||||
| {"importable_path": self.importable_path[len("$/public") :]}, | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def _short_content_hash(path: Path) -> str: | ||||||||||||
| """Get a short content hash for an asset file. | ||||||||||||
|
|
||||||||||||
| Args: | ||||||||||||
| path: The file to hash. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| The first 8 hex characters of the file's SHA-256 hash. | ||||||||||||
| """ | ||||||||||||
| digest = hashlib.sha256() | ||||||||||||
| for _ in range(_MAX_HASH_ATTEMPTS): | ||||||||||||
| digest = hashlib.sha256() | ||||||||||||
| start_stat = path.stat() | ||||||||||||
| with path.open("rb") as file: | ||||||||||||
| while chunk := file.read(_HASH_CHUNK_SIZE): | ||||||||||||
| digest.update(chunk) | ||||||||||||
| end_stat = path.stat() | ||||||||||||
| if ( | ||||||||||||
| start_stat.st_size == end_stat.st_size | ||||||||||||
| and start_stat.st_mtime_ns == end_stat.st_mtime_ns | ||||||||||||
| ): | ||||||||||||
| break | ||||||||||||
| return digest.hexdigest()[:8] | ||||||||||||
|
Comment on lines
+141
to
+142
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
we should consider doing something reasonable when we can't get a stable hash, like using the compile timestamp as the hash rather than always returning the last digest, which might cause the browser to load a previous cached version. at least if we take the timestamp, the browser will reload the asset once per compile, which is probably good enough for most uses. |
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def _versioned_asset_path(relative_path: str, source_file: Path) -> AssetPathStr: | ||||||||||||
| """Create an asset URL with a content-hash query parameter. | ||||||||||||
|
|
||||||||||||
| Args: | ||||||||||||
| relative_path: The unprefixed public asset path. | ||||||||||||
| source_file: The source file used to calculate the content hash. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| A versioned asset URL that preserves an unversioned import path. | ||||||||||||
| """ | ||||||||||||
| versioned_path = f"{relative_path}?v={_short_content_hash(source_file)}" | ||||||||||||
|
greptile-apps[bot] marked this conversation as resolved.
masenf marked this conversation as resolved.
|
||||||||||||
| return AssetPathStr(versioned_path, importable_path=relative_path) | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def remove_stale_external_asset_symlinks(): | ||||||||||||
|
|
@@ -176,7 +242,10 @@ def asset( | |||||||||||
| if not backend_only and not src_file_local.exists(): | ||||||||||||
| msg = f"File not found: {src_file_local}" | ||||||||||||
| raise FileNotFoundError(msg) | ||||||||||||
| return AssetPathStr(f"/{path}") | ||||||||||||
| relative_path = f"/{path}" | ||||||||||||
| if backend_only and not src_file_local.exists(): | ||||||||||||
| return AssetPathStr(relative_path) | ||||||||||||
| return _versioned_asset_path(relative_path, src_file_local) | ||||||||||||
|
|
||||||||||||
| # Shared asset handling | ||||||||||||
| # Determine the file by which the asset is exposed. | ||||||||||||
|
|
@@ -212,4 +281,7 @@ def asset( | |||||||||||
| dst_file.unlink() | ||||||||||||
| dst_file.symlink_to(src_file_shared) | ||||||||||||
|
|
||||||||||||
| return AssetPathStr(f"/{external}/{subfolder}/{path}") | ||||||||||||
| return _versioned_asset_path( | ||||||||||||
| f"/{external}/{subfolder}/{path}", | ||||||||||||
| src_file_shared, | ||||||||||||
| ) | ||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like a useless extra initialization; just keep the one in the loop