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

rx._x.asset improvements #3624

Merged
merged 14 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 58 additions & 27 deletions reflex/experimental/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,55 +5,86 @@
from typing import Optional

from reflex import constants
from reflex.utils.exec import is_backend_only


def asset(relative_filename: str, subfolder: Optional[str] = None) -> str:
"""Add an asset to the app.
def asset(
path: str,
shared: bool = False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the shared argument is a little bit confusing to me... is shared=False for referencing files that are already in the app's assets/ directory?

i suppose this is to align the API to work with source-relative assets as well as assets that are already in the correct place? But setting the default to False seems to break how existing callers are already using this function to reference source-relative assets.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shared=True ist for librarys which want to expose the assets to other python modules/reflex apps. Shared mode expects the files to be relative to the python source file where rx._x.asset is called.

shared=False is for "normal"/"local" assets in reflex. It just returns the serving path and checks if the file really exits.

subfolder: Optional[str] = None,
) -> str:
"""Add an asset to the app, either shared as a symlink or local.

Shared/External/Library assets:
Place the file next to your including python file.
Copies the file to the app's external assets directory.
Links the file to the app's external assets directory.

Example:
```python
rx.script(src=rx._x.asset("my_custom_javascript.js"))
rx.image(src=rx._x.asset("test_image.png","subfolder"))
# my_custom_javascript.js is a shared asset located next to the including python file.
rx.script(src=rx._x.asset(path="my_custom_javascript.js", shared=True))
rx.image(src=rx._x.asset(path="test_image.png", shared=True, subfolder="subfolder"))
```

Local/Internal assets:
Place the file in the app's assets/ directory.

Example:
```python
# local_image.png is an asset located in the app's assets/ directory. It cannot be shared when developing a library.
rx.image(src=rx._x.asset(path="local_image.png"))
```

Args:
relative_filename: The relative filename of the asset.
subfolder: The directory to place the asset in.
path: The relative path of the asset.
subfolder: The directory to place the shared asset in.
shared: Whether to expose the asset to other apps.

Raises:
FileNotFoundError: If the file does not exist.
ValueError: If the module is None.
ValueError: If subfolder is provided for local assets.

Returns:
The relative URL to the copied asset.
The relative URL to the asset.
"""
assets = constants.Dirs.APP_ASSETS
backend_only = is_backend_only()

# Local asset handling
if not shared:
cwd = Path.cwd()
src_file_local = cwd / assets / path
if subfolder is not None:
raise ValueError("Subfolder is not supported for local assets.")
if not backend_only and not src_file_local.exists():
raise FileNotFoundError(f"File not found: {src_file_local}")
return f"/{path}"

# Shared asset handling
# Determine the file by which the asset is exposed.
calling_file = inspect.stack()[1].filename
module = inspect.getmodule(inspect.stack()[1][0])
if module is None:
raise ValueError("Module is None")
caller_module_path = module.__name__.replace(".", "/")

subfolder = f"{caller_module_path}/{subfolder}" if subfolder else caller_module_path

src_file = Path(calling_file).parent / relative_filename
assert module is not None

assets = constants.Dirs.APP_ASSETS
external = constants.Dirs.EXTERNAL_APP_ASSETS
src_file_shared = Path(calling_file).parent / path
if not src_file_shared.exists():
raise FileNotFoundError(f"File not found: {src_file_shared}")

if not src_file.exists():
raise FileNotFoundError(f"File not found: {src_file}")
caller_module_path = module.__name__.replace(".", "/")
subfolder = f"{caller_module_path}/{subfolder}" if subfolder else caller_module_path

# Create the asset folder in the currently compiling app.
asset_folder = Path.cwd() / assets / external / subfolder
asset_folder.mkdir(parents=True, exist_ok=True)
# Symlink the asset to the app's external assets directory if running frontend.
if not backend_only:
# Create the asset folder in the currently compiling app.
asset_folder = Path.cwd() / assets / external / subfolder
asset_folder.mkdir(parents=True, exist_ok=True)

dst_file = asset_folder / relative_filename
dst_file = asset_folder / path

if not dst_file.exists():
dst_file.symlink_to(src_file)
if not dst_file.exists() and (
not dst_file.is_symlink() or dst_file.resolve() != src_file_shared.resolve()
):
dst_file.symlink_to(src_file_shared)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes me a bit nervous because symlinks are not generally enabled or available to unprivileged users on windows.

can we have a fallback path that just copies the files on windows or if this operation fails

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR did not introduce the symlinks, they were there before. i can address it in another PR


asset_url = f"/{external}/{subfolder}/{relative_filename}"
return asset_url
return f"/{external}/{subfolder}/{path}"
57 changes: 51 additions & 6 deletions tests/units/experimental/test_assets.py
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
import shutil
from pathlib import Path
from typing import Generator

import pytest

import reflex as rx
import reflex.constants as constants


def test_asset():
# Test the asset function.

def test_shared_asset() -> None:
"""Test shared assets."""
# The asset function copies a file to the app's external assets directory.
asset = rx._x.asset("custom_script.js", "subfolder")
asset = rx._x.asset(path="custom_script.js", shared=True, subfolder="subfolder")
assert asset == "/external/test_assets/subfolder/custom_script.js"
result_file = Path(
Path.cwd(), "assets/external/test_assets/subfolder/custom_script.js"
)
assert result_file.exists()

# Running a second time should not raise an error.
asset = rx._x.asset("custom_script.js", "subfolder")
asset = rx._x.asset(path="custom_script.js", shared=True, subfolder="subfolder")

# Test the asset function without a subfolder.
asset = rx._x.asset("custom_script.js")
asset = rx._x.asset(path="custom_script.js", shared=True)
assert asset == "/external/test_assets/custom_script.js"
result_file = Path(Path.cwd(), "assets/external/test_assets/custom_script.js")
assert result_file.exists()
Expand All @@ -34,3 +35,47 @@ def test_asset():

# Nothing is done to assets when file does not exist.
assert not Path(Path.cwd() / "assets/external").exists()


@pytest.mark.parametrize(
"path,shared",
[
pytest.param("non_existing_file", True),
pytest.param("non_existing_file", False),
],
)
def test_invalid_assets(path: str, shared: bool) -> None:
"""Test that asset raises an error when the file does not exist.

Args:
path: The path to the asset.
shared: Whether the asset should be shared.
"""
with pytest.raises(FileNotFoundError):
_ = rx._x.asset(path, shared=shared)


@pytest.fixture
def custom_script_in_asset_dir() -> Generator[Path, None, None]:
"""Create a custom_script.js file in the app's assets directory.

Yields:
The path to the custom_script.js file.
"""
asset_dir = Path.cwd() / constants.Dirs.APP_ASSETS
asset_dir.mkdir(exist_ok=True)
path = asset_dir / "custom_script.js"
path.touch()
yield path
path.unlink()


def test_local_asset(custom_script_in_asset_dir: Path) -> None:
"""Test that no error is raised if shared is set and both files exist.

Args:
custom_script_in_asset_dir: Fixture that creates a custom_script.js file in the app's assets directory.

"""
asset = rx._x.asset("custom_script.js", shared=False)
assert asset == "/custom_script.js"
Loading