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

refactor: fix context vars missing in isolated slot #455

Merged

Conversation

JuroOravec
Copy link
Collaborator

Closes #454

@JuroOravec JuroOravec force-pushed the jo-fix-slot-isolation-ctx-top-lvl branch from c47460d to c55016e Compare April 24, 2024 19:31
@@ -19,11 +20,17 @@
_FILLED_SLOTS_CONTENT_CONTEXT_KEY = "_DJANGO_COMPONENTS_FILLED_SLOTS"
_OUTER_ROOT_CTX_CONTEXT_KEY = "_DJANGO_COMPONENTS_OUTER_ROOT_CTX"
_SLOT_COMPONENT_ASSOC_KEY = "_DJANGO_COMPONENTS_SLOT_COMP_ASSOC"
_PARENT_COMP_KEY = "_DJANGO_COMPONENTS_PARENT_COMP"
_CURRENT_COMP_KEY = "_DJANGO_COMPONENTS_CURRENT_COMP"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On the Context object we now store the ID of the current component node that we're rendering, as well as it's parent.

The parent info is used in this MR as an indicator of whether the rendered component node is a top-level node, or it is inside another component.

If I remember right, the info on current component ID is / will be used to fix #453

outer_ctx
and not context.get(_PARENT_COMP_KEY)
and app_settings.SLOT_CONTEXT_BEHAVIOR == SlotContextBehavior.ISOLATED
and _OUTER_ROOT_CTX_CONTEXT_KEY in context # <-- Added to avoid breaking tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As the comment says, the last line was added to avoid breaking other tests (not quite sure why)

@dylanjcastillo
Copy link
Collaborator

Nice fix. But really looking forward to merging the two options and just keeping isolated and django 😄

@JuroOravec JuroOravec merged commit ae22eff into EmilStenstrom:master Apr 25, 2024
6 checks passed
@JuroOravec JuroOravec deleted the jo-fix-slot-isolation-ctx-top-lvl branch April 25, 2024 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Context vars missing in slots when isolated settings
2 participants