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

External assets #3220

Merged
merged 21 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ce72c31
feat: wip rx.asset
abulvenz May 3, 2024
40bd7ab
fix: Removed test exception. Using contemporary cwd.
abulvenz May 3, 2024
3cb68c3
fix: Adding much logic to provide a nice API for the caller.
abulvenz May 3, 2024
38c127d
fix: cleaning up comments.
abulvenz May 3, 2024
c4929ec
fix: The pipeline made me do this.
abulvenz May 3, 2024
73e5a04
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 6, 2024
3806aec
feat: Review comments. Built in symlinks on non-Windows.
abulvenz May 6, 2024
4b80aa4
fix: Using contemporary python libraries.
abulvenz May 6, 2024
76cf9d7
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 7, 2024
592ef9a
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 15, 2024
8cbd3af
feat: Added test. Moving function to _x namespace.
abulvenz May 15, 2024
2bc36f9
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 15, 2024
381b732
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 16, 2024
69b8037
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 17, 2024
f46c3c8
fix: Fixing typing for python 3.8 in hooks.
abulvenz May 17, 2024
635e701
fix: Fixing typing for python 3.8 in hooks.
abulvenz May 17, 2024
a758e33
fix: Fixing typing for python 3.8 in client_state.
abulvenz May 17, 2024
627c91b
fix: Just saw it was alpha-sorted before.
abulvenz May 17, 2024
c5cc4c1
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 21, 2024
ca09e38
fix: Removing superficial Path constructor and inlining variable.
abulvenz May 21, 2024
9972a35
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 22, 2024
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
2 changes: 1 addition & 1 deletion reflex/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
_MAPPING = {
"reflex.experimental": ["_x"],
"reflex.admin": ["admin", "AdminDash"],
"reflex.app": ["app", "App", "UploadFile"],
"reflex.app": ["app", "App", "UploadFile", "asset"],
"reflex.base": ["base", "Base"],
"reflex.compiler": ["compiler"],
"reflex.compiler.utils": ["get_asset_path"],
Expand Down
1 change: 1 addition & 0 deletions reflex/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ from reflex.admin import AdminDash as AdminDash
from reflex import app as app
from reflex.app import App as App
from reflex.app import UploadFile as UploadFile
from reflex.app import asset as asset
from reflex import base as base
from reflex.base import Base as Base
from reflex import compiler as compiler
Expand Down
40 changes: 40 additions & 0 deletions reflex/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@
import contextlib
import copy
import functools
import inspect
import io
import multiprocessing
import os
import platform
import shutil
from pathlib import Path
from typing import (
Any,
AsyncIterator,
Expand Down Expand Up @@ -1317,3 +1320,40 @@ async def on_ping(self, sid):
"""
# Emit the test event.
await self.emit(str(constants.SocketEvent.PING), "pong", to=sid)


def asset(relative_filename: str, dir: str = ".") -> str:
"""Add an asset to the app.
Place the file next to your including python file.
Copies the file to the app's external assets directory.

Example:
```python
rx.script(src=rx.asset("my_custom_javascript.js"))
rx.image(src=rx.asset("test_image.png","subfolder"))
```

Args:
relative_filename: The relative filename of the asset.
dir: The directory to place the asset in.

Returns:
The relative URL to the copied asset.

"""
calling_file = inspect.stack()[1].filename
cwd = Path.cwd()

module = inspect.getmodule(inspect.stack()[1][0])
assert module is not None

caller_module_path = module.__name__.replace(".", "/")
dir = caller_module_path if dir == "." else caller_module_path + "/" + dir
assets = constants.Dirs.APP_ASSETS
external = constants.Dirs.EXTERNAL_APP_ASSETS
Path(Path(cwd) / assets / external / dir).mkdir(parents=True, exist_ok=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a variable for Path(Path(cwd) / assets / external / dir) first and use it in the next lines. Also check if the source exists with pathlib's .resolve() method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Lendemor I hope you are fine with using symlinks here, since we are not linking directories. That seems to be the only difference. https://docs.python.org/3/library/pathlib.html#pathlib.Path.symlink_to

@benedikt-bartscher I just tested what happens when src_file is not present. It already complains No such file or directory. OK, or wrap it into a an AssetNotPresent?

Copy link
Contributor

Choose a reason for hiding this comment

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

@abulvenz awesome, if there is already a FileNotFound exception, I think we are fine, no additional wrapping needed

Copy link
Collaborator

Choose a reason for hiding this comment

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

@abulvenz Sorry for delay in review, was on break for a few days.

It's passing windows CI so it should be fine I think.

I'm not sure about adding it inside app.py however, so could you move it to it's own file? Maybe inside reflex/experimental so we'll get some time to test it in real situations before making it official.

We'll also need to add unit tests for it.

Copy link
Contributor Author

@abulvenz abulvenz May 15, 2024

Choose a reason for hiding this comment

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

Thank you @Lendemor, After v0.5.0 everybody earned a small break 🚀 . I will move it to experimental and try to add a unit test.

shutil.copy2(
Path(calling_file).parent / relative_filename,
Path(cwd) / assets / external / dir / relative_filename,
)
return "/" + external + "/" + dir + "/" + relative_filename
2 changes: 2 additions & 0 deletions reflex/constants/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class Dirs(SimpleNamespace):
WEB = ".web"
# The name of the assets directory.
APP_ASSETS = "assets"
# The name of the assets directory for external ressource (a subfolder of APP_ASSETS).
EXTERNAL_APP_ASSETS = "external"
# The name of the utils file.
UTILS = "utils"
# The name of the output static directory.
Expand Down
5 changes: 5 additions & 0 deletions reflex/constants/base.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@ from types import SimpleNamespace
from platformdirs import PlatformDirs

IS_WINDOWS = platform.system() == "Windows"
IS_WINDOWS_BUN_SUPPORTED_MACHINE = IS_WINDOWS and platform.machine() in [
"AMD64",
"x86_64",
]

class Dirs(SimpleNamespace):
WEB = ".web"
APP_ASSETS = "assets"
EXTERNAL_APP_ASSETS = "external"
UTILS = "utils"
STATIC = "_static"
STATE_PATH = "/".join([UTILS, "state"])
Expand Down
Loading