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.

Sign up for GitHub

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

Jump to bottom

Client-Side State Management for seamless user interactions #138

Open
wants to merge 1 commit into
base: localstate-management
Choose a base branch
from
Open

Client-Side State Management for seamless user interactions #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions app-examples/gallery/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
*.db
*.py[cod]
*.web
.web
__pycache__/
4 changes: 2 additions & 2 deletions nextpy/backend/vars.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1473,8 +1473,8 @@ def _var_set_state(self, state: Type[BaseState] | str) -> Any:
state_name = state if isinstance(state, str) else state.get_full_name()
new_var_data = VarData(
state=state_name,
hooks={
"const {0} = useContext(StateContexts.{0})".format(
hooks = {
"const {0} = useContext(StateContexts.{0});\nconst dispatchers = useContext(DispatchContext)".format(
format.format_state_name(state_name)
)
},
Expand Down
2 changes: 2 additions & 0 deletions nextpy/frontend/components/component.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1590,6 +1590,8 @@ def _get_memoized_event_triggers(
if isinstance(rendered_chain, str):
rendered_chain = rendered_chain.strip("{}")

rendered_chain = rendered_chain+'}'
Copy link

Choose a reason for hiding this comment

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

Why is this change made? Isn't rendered_chain working correctly previously?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've noticed an issue with the original rendered_chain definition in the component.py file. The initial code is as follows:

{(_e0) =>{ 
    const dispatcher = dispatchers['state.state'];
    dispatcher({ message: _e0.target.value });
    addEvents([Event("state.state.set_message", {value:_e0.target.value})], _e0, {}) 
}}

However, there seems to be a processing step that alters rendered_chain by removing the opening and closing curly brackets {}. This modification necessitates manually appending an additional closing curly bracket } at the end of the rendered_chain to ensure correct rendering in the index.js file. The modified code looks like this:

_e0 => {
    const dispatcher = dispatchers['state.state']
    dispatcher({ message: _e0.target.value })
    addEvents([Event('state.state.set_message', { value: _e0.target.value })], _e0, {})
},

This workaround fixes the rendering issue.


# Hash the rendered EventChain to get a deterministic function name.
chain_hash = md5(str(rendered_chain).encode("utf-8")).hexdigest()
memo_name = f"{event_trigger}_{chain_hash}"
Comment on lines 1590 to 1597
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-1594]

The file introduces a comprehensive framework for defining and rendering components in a Next.js application, leveraging React and custom logic for state management, event handling, and dynamic rendering. The implementation of StatefulComponent and its memoization strategy is particularly noteworthy, aiming to optimize re-renders and improve performance. However, ensure that the memoization logic correctly handles all edge cases, especially concerning dynamic content and event handlers, to prevent stale renders or incorrect behavior. Additionally, the extensive use of dynamic typing and reflection could impact maintainability and type safety; consider leveraging TypeScript or more explicit type annotations for critical paths in the codebase.

Expand Down
1 change: 1 addition & 0 deletions nextpy/frontend/templates/jinja/web/pages/base_page.js.jinja2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Purpose:
{# ==============================
LIBRARY IMPORTS BLOCK
============================== #}
import { DispatchContext } from 'utils/context'
Copy link

Choose a reason for hiding this comment

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

Instead of doing it here, we should move this to default imports which are being used by the jinja template from utils.py file (most probably)

{#
Purpose:
- Renders all required library imports for the current page or component.
Expand Down
23 changes: 22 additions & 1 deletion nextpy/utils/format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,28 @@ def format_prop(

chain = ",".join([format_event(event) for event in prop.events])
event = f"addEvents([{chain}], {arg_def}, {json_dumps(prop.event_actions)})"
prop = f"{arg_def} => {event}"

if isinstance(event, str):
event = event.strip("{}")

parts = chain.split('.')
formatted_chain = f'{parts[0]} . {parts[1]}.{parts[2]}'

# Extract "_e0.target.value"
value_match = re.search(r"value:([^,)}]+)", event)
if value_match:
value = value_match.group(1)
Copy link

Choose a reason for hiding this comment

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

Would .group(1) call always return something?



# Extract "state.state"
message_match = re.search(r"addEvents\(\[\S+?\(\"([^.]+?\.[^.]+)", event)
if message_match:
message = message_match.group(1)

dispatcher_line = f"const dispatcher = dispatchers['{message}'];\n" \
f"dispatcher({{ message: {value} }});"

prop = f"{arg_def} =>{{ {dispatcher_line}\n{event} }}"
Comment on lines 351 to +373
Copy link
Contributor

Choose a reason for hiding this comment

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

The implementation of event handling within the format_prop function introduces several issues and areas for improvement:

  1. Complexity: The function has become significantly more complex with the addition of event handling logic. Consider refactoring to improve readability and maintainability.
  2. Hardcoded Values: The construction of formatted_chain (line 357) seems to assume a specific structure for chain. This might not be robust against different event structures.
  3. Regex Usage: The use of regex to extract values (lines 360-362 and 366-368) could be error-prone and might not work as expected for all possible event formats.
  4. Dispatcher Line Construction: The construction of dispatcher_line (lines 370-371) directly within the function increases its responsibilities beyond formatting props. Consider separating event dispatching logic into a dedicated function or module.

Consider refactoring the event handling logic to address these concerns, potentially by separating concerns and simplifying the implementation.

Comment on lines +353 to +373
Copy link

Choose a reason for hiding this comment

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

Can you add example as comment for each part which details the input and output from the executed code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure 👍


# Handle other types.
elif isinstance(prop, str):
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "nextpy"
version = "0.3.3"
version = "0.3.5"
anubrag marked this conversation as resolved.
Show resolved Hide resolved
description = "⚡The Pure Python Framework for Web Apps, Meticulously Optimized for 🤖AI agents🤖. World's first AMS🥇"
license = "Apache-2.0"
authors = ["Team dotagent <[email protected]>"]
Expand Down